Add history and persistence support to Evaluator.
Bug: 31623549
Bug: 31686717
This version superficially works, but introduces a number of FIXME
comments that should be repaired before shipping. It may not stand
up well to monkey tests.
Add ExpressionDB.java that encapsulates a SQLite database to hold
the calculator history.
Change Evaluator to support simultaneous evaluation of many different
expressions, such as those in the history. In order to avoid UI thread
blocking, the evaluator now loads referenced subexpression on demand,
and multiple expressions can be evaluated simultaneously. We handle
the concurrency somewhat optimistically, arranging to have concurrent
evaluations not step on each other, at the expense of occasionally
throwing away a redundant result.
Change the expression serialization algorithm to make formulas more
compact. Do this now to minimize annoying database format changes.
Persist the saved clipboard and "memory" state.
Add two buttons in landscape mode to allow minimal access to "memory".
Since this involved a substantial rewrite of Evaluator.java, it
includes some drive-by cleanups and minor bug fixes.
Notably:
mMsdIndex was apparently always recomputed, but never actually
saved. Oops. This no doubt added a small amount of overhead.
We updated the decimal conversion size limits, hopefully addressing
b/30200325, possibly at some expense of slightly worse behavior
on old 32-bit devices.
Tests: Tested manually with a bit of added instrumentation.
Ran existing automated tests.
Change-Id: Ifae408d7b4b6cacd19f0e8f5aca146e9c653927e
diff --git a/src/com/android/calculator2/CalculatorExpr.java b/src/com/android/calculator2/CalculatorExpr.java
index 41dfe13..f20d2bf 100644
--- a/src/com/android/calculator2/CalculatorExpr.java
+++ b/src/com/android/calculator2/CalculatorExpr.java
@@ -22,7 +22,6 @@
import android.text.Spanned;
import android.text.style.TtsSpan;
import android.text.style.TtsSpan.TextBuilder;
-import android.util.Log;
import java.math.BigInteger;
import java.io.DataInput;
@@ -47,6 +46,35 @@
* when reading it back in.
*/
class CalculatorExpr {
+ /**
+ * An interface for resolving expression indices in embedded subexpressions to
+ * the associated CalculatorExpr, and associating a UnifiedReal result with it.
+ * All methods are thread-safe in the strong sense; they may be called asynchronously
+ * at any time from any thread.
+ */
+ public interface ExprResolver {
+ /*
+ * Retrieve the expression corresponding to index.
+ */
+ CalculatorExpr getExpr(long index);
+ /*
+ * Retrive the degree mode associated with the expression at index i.
+ */
+ boolean getDegreeMode(long index);
+ /*
+ * Retrieve the stored result for the expression at index, or return null.
+ */
+ UnifiedReal getResult(long index);
+ /*
+ * Atomically test for an existing result, and set it if there was none.
+ * Return the prior result if there was one, or the new one if there was not.
+ * May only be called after getExpr.
+ */
+ UnifiedReal putResultIfAbsent(long index, UnifiedReal result);
+ // FIXME: Check that long timeouts for embedded expressions are propagated
+ // correctly.
+ }
+
private ArrayList<Token> mExpr; // The actual representation
// as a list of tokens. Constant
// tokens are always nonempty.
@@ -60,7 +88,9 @@
abstract TokenKind kind();
/**
- * Write kind as Byte followed by data needed by subclass constructor.
+ * Write token as either a very small Byte containing the TokenKind,
+ * followed by data needed by subclass constructor,
+ * or as a byte >= 0x20 directly describing the OPERATOR token.
*/
abstract void write(DataOutput out) throws IOException;
@@ -77,17 +107,17 @@
* Representation of an operator token
*/
private static class Operator extends Token {
+ // TODO: rename id.
public final int id; // We use the button resource id
Operator(int resId) {
id = resId;
}
- Operator(DataInput in) throws IOException {
- id = in.readInt();
+ Operator(byte op) throws IOException {
+ id = KeyMaps.fromByte(op);
}
@Override
void write(DataOutput out) throws IOException {
- out.writeByte(TokenKind.OPERATOR.ordinal());
- out.writeInt(id);
+ out.writeByte(KeyMaps.toByte(id));
}
@Override
public CharSequence toCharSequence(Context context) {
@@ -114,28 +144,44 @@
private String mWhole; // String preceding decimal point.
private String mFraction; // String after decimal point.
private int mExponent; // Explicit exponent, only generated through addExponent.
+ private static int SAW_DECIMAL = 0x1;
+ private static int HAS_EXPONENT = 0x2;
Constant() {
mWhole = "";
mFraction = "";
- mSawDecimal = false;
- mExponent = 0;
+ // mSawDecimal = false;
+ // mExponent = 0;
};
Constant(DataInput in) throws IOException {
mWhole = in.readUTF();
- mSawDecimal = in.readBoolean();
- mFraction = in.readUTF();
- mExponent = in.readInt();
+ byte flags = in.readByte();
+ if ((flags & SAW_DECIMAL) != 0) {
+ mSawDecimal = true;
+ mFraction = in.readUTF();
+ } else {
+ // mSawDecimal = false;
+ mFraction = "";
+ }
+ if ((flags & HAS_EXPONENT) != 0) {
+ mExponent = in.readInt();
+ }
}
@Override
void write(DataOutput out) throws IOException {
+ byte flags = (byte)((mSawDecimal ? SAW_DECIMAL : 0)
+ | (mExponent != 0 ? HAS_EXPONENT : 0));
out.writeByte(TokenKind.CONSTANT.ordinal());
out.writeUTF(mWhole);
- out.writeBoolean(mSawDecimal);
- out.writeUTF(mFraction);
- out.writeInt(mExponent);
+ out.writeByte(flags);
+ if (mSawDecimal) {
+ out.writeUTF(mFraction);
+ }
+ if (mExponent != 0) {
+ out.writeInt(mExponent);
+ }
}
// Given a button press, append corresponding digit.
@@ -266,105 +312,43 @@
}
}
- // Hash maps used to detect duplicate subexpressions when we write out CalculatorExprs and
- // read them back in.
- private static final ThreadLocal<IdentityHashMap<UnifiedReal, Integer>>outMap =
- new ThreadLocal<IdentityHashMap<UnifiedReal, Integer>>();
- // Maps expressions to indices on output
- private static final ThreadLocal<HashMap<Integer, PreEval>>inMap =
- new ThreadLocal<HashMap<Integer, PreEval>>();
- // Maps expressions to indices on output
- private static final ThreadLocal<Integer> exprIndex = new ThreadLocal<Integer>();
-
- /**
- * Prepare for expression output.
- * Initializes map that will lbe used to avoid duplicating shared subexpressions.
- * This avoids a potential exponential blow-up in the expression size.
- */
- public static void initExprOutput() {
- outMap.set(new IdentityHashMap<UnifiedReal, Integer>());
- exprIndex.set(Integer.valueOf(0));
- }
-
- /**
- * Prepare for expression input.
- * Initializes map that will be used to reconstruct shared subexpressions.
- */
- public static void initExprInput() {
- inMap.set(new HashMap<Integer, PreEval>());
- }
-
/**
* The "token" class for previously evaluated subexpressions.
* We treat previously evaluated subexpressions as tokens. These are inserted when we either
* continue an expression after evaluating some of it, or copy an expression and paste it back
* in.
+ * This only contains enough information to allow us to display the expression in a
+ * formula, or reevaluate the expression with the aid of an ExprResolver; we no longer
+ * cache the result. The expression corresponding to the index can be obtained through
+ * the ExprResolver, which looks it up in a subexpression database.
* The representation includes a UnifiedReal value. In order to
* support saving and restoring, we also include the underlying expression itself, and the
* context (currently just degree mode) used to evaluate it. The short string representation
* is also stored in order to avoid potentially expensive recomputation in the UI thread.
*/
private static class PreEval extends Token {
- public final UnifiedReal value;
- private final CalculatorExpr mExpr;
- private final EvalContext mContext;
+ public final long mIndex;
private final String mShortRep; // Not internationalized.
- PreEval(UnifiedReal val, CalculatorExpr expr, EvalContext ec, String shortRep) {
- value = val;
- mExpr = expr;
- mContext = ec;
+ PreEval(long index, String shortRep) {
+ mIndex = index;
mShortRep = shortRep;
}
- // In writing out PreEvals, we are careful to avoid writing out duplicates. We conclude
- // that two expressions are duplicates if they have the same UnifiedReal value. This
- // avoids a potential exponential blow up in certain off cases and redundant evaluation
- // after reading them back in. The parameter hash map maps expressions we've seen
- // before to their index.
@Override
+ // This writes out only a shallow representation of the result, without
+ // information about subexpressions. To write out a deep representation, we
+ // find referenced subexpressions, and iteratively write those as well.
public void write(DataOutput out) throws IOException {
out.writeByte(TokenKind.PRE_EVAL.ordinal());
- Integer index = outMap.get().get(value);
- if (index == null) {
- int nextIndex = exprIndex.get() + 1;
- exprIndex.set(nextIndex);
- outMap.get().put(value, nextIndex);
- out.writeInt(nextIndex);
- mExpr.write(out);
- mContext.write(out);
- out.writeUTF(mShortRep);
- } else {
- // Just write out the index
- out.writeInt(index);
+ if (mIndex > Integer.MAX_VALUE || mIndex < Integer.MIN_VALUE) {
+ // This would be millions of expressions per day for the life of the device.
+ throw new AssertionError("Expression index too big");
}
+ out.writeInt((int)mIndex);
+ out.writeUTF(mShortRep);
}
PreEval(DataInput in) throws IOException {
- int index = in.readInt();
- PreEval prev = inMap.get().get(index);
- if (prev == null) {
- mExpr = new CalculatorExpr(in);
- mContext = new EvalContext(in, mExpr.mExpr.size());
- // Recompute other fields We currently do this in the UI thread, but we only
- // create PreEval expressions that were previously successfully evaluated, and
- // thus don't diverge. We also only evaluate to a constructive real, which
- // involves substantial work only in fairly contrived circumstances.
- // TODO: Deal better with slow evaluations.
- EvalRet res = null;
- try {
- res = mExpr.evalExpr(0, mContext);
- } catch (SyntaxException e) {
- // Should be impossible, since we only write out
- // expressions that can be evaluated.
- Log.e("Calculator", "Unexpected syntax exception" + e);
- }
- value = res.val;
- mShortRep = in.readUTF();
- inMap.get().put(index, this);
- } else {
- value = prev.value;
- mExpr = prev.mExpr;
- mContext = prev.mContext;
- mShortRep = prev.mShortRep;
- }
+ mIndex = in.readInt();
+ mShortRep = in.readUTF();
}
@Override
public CharSequence toCharSequence(Context context) {
@@ -383,15 +367,18 @@
* Read token from in.
*/
public static Token newToken(DataInput in) throws IOException {
- TokenKind kind = tokenKindValues[in.readByte()];
- switch(kind) {
- case CONSTANT:
- return new Constant(in);
- case OPERATOR:
- return new Operator(in);
- case PRE_EVAL:
- return new PreEval(in);
- default: throw new IOException("Bad save file format");
+ byte kindByte = in.readByte();
+ if (kindByte < 0x20) {
+ TokenKind kind = tokenKindValues[kindByte];
+ switch(kind) {
+ case CONSTANT:
+ return new Constant(in);
+ case PRE_EVAL:
+ return new PreEval(in);
+ default: throw new IOException("Bad save file format");
+ }
+ } else {
+ return new Operator(kindByte);
}
}
@@ -441,7 +428,7 @@
/**
* Does this expression end with a binary operator?
*/
- private boolean hasTrailingBinary() {
+ boolean hasTrailingBinary() {
int s = mExpr.size();
if (s == 0) return false;
Token t = mExpr.get(s-1);
@@ -612,14 +599,13 @@
/**
* Return a new expression consisting of a single token representing the current pre-evaluated
* expression.
- * The caller supplies the value, degree mode, and short string representation, which must
- * have been previously computed. Thus this is guaranteed to terminate reasonably quickly.
+ * The caller supplies the expression index and short string representation.
+ * The expression must have been previously evaluated.
*/
- public CalculatorExpr abbreviate(UnifiedReal val, boolean dm, String sr) {
+ public CalculatorExpr abbreviate(long index, String sr) {
CalculatorExpr result = new CalculatorExpr();
@SuppressWarnings("unchecked")
- Token t = new PreEval(val, new CalculatorExpr((ArrayList<Token>) mExpr.clone()),
- new EvalContext(dm, mExpr.size()), sr);
+ Token t = new PreEval(index, sr);
result.mExpr.add(t);
return result;
}
@@ -644,14 +630,17 @@
private static class EvalContext {
public final int mPrefixLength; // Length of prefix to evaluate. Not explicitly saved.
public final boolean mDegreeMode;
+ public final ExprResolver mExprResolver; // Reconstructed, not saved.
// If we add any other kinds of evaluation modes, they go here.
- EvalContext(boolean degreeMode, int len) {
+ EvalContext(boolean degreeMode, int len, ExprResolver er) {
mDegreeMode = degreeMode;
mPrefixLength = len;
+ mExprResolver = er;
}
- EvalContext(DataInput in, int len) throws IOException {
+ EvalContext(DataInput in, int len, ExprResolver er) throws IOException {
mDegreeMode = in.readBoolean();
mPrefixLength = len;
+ mExprResolver = er;
}
void write(DataOutput out) throws IOException {
out.writeBoolean(mDegreeMode);
@@ -714,8 +703,16 @@
return new EvalRet(i+1,new UnifiedReal(c.toRational()));
}
if (t instanceof PreEval) {
- final PreEval p = (PreEval)t;
- return new EvalRet(i+1, p.value);
+ final long index = ((PreEval)t).mIndex;
+ UnifiedReal res = ec.mExprResolver.getResult(index);
+ if (res == null) {
+ CalculatorExpr nestedExpr = ec.mExprResolver.getExpr(index);
+ EvalContext newEc = new EvalContext(ec.mExprResolver.getDegreeMode(index),
+ nestedExpr.trailingBinaryOpsStart(), ec.mExprResolver);
+ EvalRet new_res = nestedExpr.evalExpr(0, newEc);
+ res = ec.mExprResolver.putResultIfAbsent(index, new_res.val);
+ }
+ return new EvalRet(i+1, res);
}
EvalRet argVal;
switch(((Operator)(t)).id) {
@@ -1007,7 +1004,7 @@
*
* @param degreeMode use degrees rather than radians
*/
- UnifiedReal eval(boolean degreeMode) throws SyntaxException
+ UnifiedReal eval(boolean degreeMode, ExprResolver er) throws SyntaxException
// And unchecked exceptions thrown by UnifiedReal, CR,
// and BoundedRational.
{
@@ -1017,7 +1014,7 @@
// expressions, and don't generate an error where we previously displayed an instant
// result. This reflects the Android L design.
int prefixLen = trailingBinaryOpsStart();
- EvalContext ec = new EvalContext(degreeMode, prefixLen);
+ EvalContext ec = new EvalContext(degreeMode, prefixLen, er);
EvalRet res = evalExpr(0, ec);
if (res.pos != prefixLen) {
throw new SyntaxException("Failed to parse full expression");