diff --git a/accord-core/src/main/java/accord/impl/CommandChange.java b/accord-core/src/main/java/accord/impl/CommandChange.java index 0b7115b0c..28fb3b232 100644 --- a/accord-core/src/main/java/accord/impl/CommandChange.java +++ b/accord-core/src/main/java/accord/impl/CommandChange.java @@ -20,6 +20,7 @@ import java.util.function.BiPredicate; import java.util.function.Function; +import java.util.function.Predicate; import java.util.function.ToLongFunction; import com.google.common.annotations.VisibleForTesting; @@ -52,13 +53,13 @@ import static accord.impl.CommandChange.Field.EXECUTES_AT_LEAST; import static accord.impl.CommandChange.Field.EXECUTE_AT; import static accord.impl.CommandChange.Field.FIELDS; +import static accord.impl.CommandChange.Field.MIN_UNIQUE_HLC; import static accord.impl.CommandChange.Field.PARTIAL_DEPS; import static accord.impl.CommandChange.Field.PARTIAL_TXN; import static accord.impl.CommandChange.Field.PARTICIPANTS; import static accord.impl.CommandChange.Field.PROMISED; import static accord.impl.CommandChange.Field.RESULT; import static accord.impl.CommandChange.Field.SAVE_STATUS; -import static accord.impl.CommandChange.Field.MIN_UNIQUE_HLC; import static accord.impl.CommandChange.Field.WAITING_ON; import static accord.impl.CommandChange.Field.WRITES; import static accord.local.Cleanup.NO; @@ -74,7 +75,11 @@ import static accord.local.Command.Truncated.invalidated; import static accord.local.Command.Truncated.vestigial; import static accord.local.StoreParticipants.Filter.LOAD; +import static accord.primitives.Known.Definition.DefinitionErased; +import static accord.primitives.Known.KnownDeps.DepsErased; import static accord.primitives.Known.KnownExecuteAt.ApplyAtKnown; +import static accord.primitives.Known.KnownExecuteAt.ExecuteAtErased; +import static accord.primitives.Known.Outcome.WasApply; import static accord.primitives.Status.Durability.NotDurable; public class CommandChange @@ -101,6 +106,37 @@ public enum Field public static final Field[] FIELDS = values(); } + /** + * SaveStatus.Known contains information about erased / nullified fields, + * which we can use in order to mark the corresponding fields as changed + * and setting them to null when they are erased. + */ + public static int[] saveStatusMasks; + + static + { + saveStatusMasks = new int[SaveStatus.values().length]; + for (int i = 0; i < saveStatusMasks.length; i++) + { + SaveStatus saveStatus = SaveStatus.forOrdinal(i); + int mask = 0; + if (forceFieldChangedToNullFlag(saveStatus, saveStatus.known::is, DepsErased)) + mask |= setFieldIsNullAndChanged(PARTIAL_DEPS, mask); + if (forceFieldChangedToNullFlag(saveStatus, saveStatus.known::is, ExecuteAtErased)) + mask |= setFieldIsNullAndChanged(EXECUTE_AT, mask); + if (forceFieldChangedToNullFlag(saveStatus, saveStatus.known::is, DefinitionErased)) + mask |= setFieldIsNullAndChanged(PARTIAL_TXN, mask); + if (forceFieldChangedToNullFlag(saveStatus, saveStatus.known::is, WasApply)) + mask |= setFieldIsNullAndChanged(RESULT, mask); + saveStatusMasks[i] = mask; + } + } + + private static boolean forceFieldChangedToNullFlag(SaveStatus saveStatus, Predicate predicate, T erased) + { + return saveStatus == SaveStatus.Vestigial || predicate.test(erased); + } + public static class Builder { protected final int mask; @@ -151,72 +187,16 @@ public Builder() this(ALL); } - public TxnId txnId() - { - return txnId; - } - - public Timestamp executeAt() - { - return executeAt; - } - - // TODO: why is this unused in BurnTest - public Timestamp executeAtLeast() - { - return executeAtLeast; - } - public SaveStatus saveStatus() { return saveStatus; } - public Status.Durability durability() - { - return durability; - } - - public Ballot acceptedOrCommitted() - { - return acceptedOrCommitted; - } - - public Ballot promised() - { - return promised; - } - public StoreParticipants participants() { return participants; } - public PartialTxn partialTxn() - { - return partialTxn; - } - - public PartialDeps partialDeps() - { - return partialDeps; - } - - public CommandChange.WaitingOnProvider waitingOn() - { - return waitingOn; - } - - public Writes writes() - { - return writes; - } - - public Result result() - { - return result; - } - public void clear() { flags = 0; @@ -286,7 +266,7 @@ public Cleanup shouldCleanup(Input input, Agent agent, RedundantBefore redundant public Builder maybeCleanup(Cleanup cleanup) { - if (saveStatus() == null) + if (saveStatus == null) return this; switch (cleanup) @@ -531,6 +511,10 @@ public static int getFlags(Command before, Command after) flags = setChanged(PARTICIPANTS, flags); flags = setChanged(SAVE_STATUS, flags); } + + if (after.saveStatus() != null) + flags |= saveStatusMasks[after.saveStatus().ordinal()]; + return flags; } diff --git a/accord-core/src/test/java/accord/impl/basic/InMemoryJournal.java b/accord-core/src/test/java/accord/impl/basic/InMemoryJournal.java index deeceb501..9807c8779 100644 --- a/accord-core/src/test/java/accord/impl/basic/InMemoryJournal.java +++ b/accord-core/src/test/java/accord/impl/basic/InMemoryJournal.java @@ -83,9 +83,8 @@ import static accord.impl.CommandChange.isNull; import static accord.impl.CommandChange.nextSetField; import static accord.impl.CommandChange.setChanged; -import static accord.impl.CommandChange.setFieldIsNull; +import static accord.impl.CommandChange.setFieldIsNullAndChanged; import static accord.impl.CommandChange.toIterableSetFields; -import static accord.impl.CommandChange.unsetFieldIsNull; import static accord.impl.CommandChange.unsetIterable; import static accord.impl.CommandChange.validateFlags; import static accord.local.Cleanup.Input.FULL; @@ -172,10 +171,10 @@ private Builder reconstruct(List saved, Load load) if (saved == null) return null; - // TODO (expected): match C* and visit in reverse order Builder builder = null; - for (Diff diff : saved) + for (int i = saved.size() - 1; i >= 0; i--) { + Diff diff = saved.get(i); if (builder == null) builder = new Builder(diff.txnId, load); builder.apply(diff); @@ -560,15 +559,21 @@ private void apply(Diff diff) { Field field = nextSetField(iterable); - this.flags = setChanged(field, this.flags); + // Since we are iterating in reverse order, we skip the fields that were + // set by entries writer later (i.e. already read ones). + if (isChanged(field, this.flags) || isNull(field, mask)) + { + iterable = unsetIterable(field, iterable); + continue; + } + if (isNull(field, diff.flags)) { - this.flags = setFieldIsNull(field, this.flags); - setNull(field); + this.flags = setFieldIsNullAndChanged(field, this.flags); } else { - this.flags = unsetFieldIsNull(field, this.flags); + this.flags = setChanged(field, this.flags); deserialize(diff, field); } @@ -576,54 +581,6 @@ private void apply(Diff diff) } } - private void setNull(Field field) - { - switch (field) - { - case EXECUTE_AT: - executeAt = null; - break; - case EXECUTES_AT_LEAST: - executeAtLeast = null; - break; - case MIN_UNIQUE_HLC: - minUniqueHlc = 0; - break; - case SAVE_STATUS: - saveStatus = null; - break; - case DURABILITY: - durability = null; - break; - case ACCEPTED: - acceptedOrCommitted = null; - break; - case PROMISED: - promised = null; - break; - case PARTICIPANTS: - participants = null; - break; - case PARTIAL_TXN: - partialTxn = null; - break; - case PARTIAL_DEPS: - partialDeps = null; - break; - case WAITING_ON: - waitingOn = null; - break; - case WRITES: - writes = null; - break; - case RESULT: - result = null; - break; - case CLEANUP: - throw new IllegalStateException(); - } - } - private void deserialize(Diff diff, Field field) { switch (field)