From 03ac2b52c0c754813b693db67354274d1ead61b1 Mon Sep 17 00:00:00 2001 From: mikera Date: Thu, 29 Aug 2024 17:34:29 +0100 Subject: [PATCH] Ref testing and refactoring for better persistence behaviour --- .../src/main/java/convex/core/data/ACell.java | 14 +++++++++-- .../src/main/java/convex/core/data/Cells.java | 9 ++++++-- .../src/main/java/convex/core/data/Ref.java | 4 ++-- .../src/main/java/convex/core/data/Sets.java | 7 +----- .../java/convex/core/data/StringShort.java | 2 +- .../main/java/convex/core/data/Strings.java | 4 ++++ .../java/convex/core/data/VectorArray.java | 2 +- .../java/convex/core/data/VectorLeaf.java | 2 +- .../src/main/java/convex/etch/Etch.java | 12 +++++----- .../src/main/java/convex/etch/EtchStore.java | 23 ++++++++----------- .../java/convex/core/data/BlocksTest.java | 7 ++++++ .../convex/core/data/CollectionsTest.java | 3 +++ .../test/java/convex/core/data/RefTest.java | 4 +++- .../java/convex/core/data/VectorsTest.java | 13 ++++++----- 14 files changed, 64 insertions(+), 42 deletions(-) diff --git a/convex-core/src/main/java/convex/core/data/ACell.java b/convex-core/src/main/java/convex/core/data/ACell.java index 6f8c2b9b9..c7c3dfe34 100644 --- a/convex-core/src/main/java/convex/core/data/ACell.java +++ b/convex-core/src/main/java/convex/core/data/ACell.java @@ -439,7 +439,10 @@ public Ref getRef(int i) { } /** - * Updates all Refs in this object using the given function. + * Updates all child Refs in this object using the given function. + * + * This clears the currently cached Ref if an update occurred. This is because, presumably, + * a new Ref for this cell needs to be created. * * The function *must not* change the hash value of Refs, in order to ensure * structural integrity of modified data structures. @@ -504,12 +507,19 @@ public void attachMemorySize(long memorySize) { } /** - * Updates the cached ref of this Cell + * Sets the cached ref of this Cell if it is not already set. USe with caution. * * @param ref Ref to assign */ @SuppressWarnings("unchecked") public void attachRef(Ref ref) { + Ref current=this.cachedRef; + if (current!=null) { + // This solves problem of trashing internal cached refs + if (ref.getStatus()<=current.getStatus()) return; + // return; + // // throw new IllegalStateException("Cell of type "+Utils.getClassName(this)+" already has cached Ref"); + } this.cachedRef=(Ref) ref; } diff --git a/convex-core/src/main/java/convex/core/data/Cells.java b/convex-core/src/main/java/convex/core/data/Cells.java index 57bae3cf7..16e197f15 100644 --- a/convex-core/src/main/java/convex/core/data/Cells.java +++ b/convex-core/src/main/java/convex/core/data/Cells.java @@ -129,7 +129,8 @@ public static boolean isValue(ACell a) { * @throws IOException */ public static T persist(T a) throws IOException { - return persist(a,Stores.current()); + AStore store=Stores.current(); + return persist(a,store); } /** @@ -247,7 +248,10 @@ public static void visitBranches(ACell a, Consumer visitor) { } /** - * Intern a Cell permanently in memory (for JVM lifetime) + * Intern a Cell permanently in memory (for JVM lifetime). + * + * SECURITY: do not do this for any generated structure from external sources. The could DoS your memory. + * * @param Type of Cell * @param value Value to intern * @return Interned Cell @@ -255,6 +259,7 @@ public static void visitBranches(ACell a, Consumer visitor) { public static T intern(T value) { Ref ref=Ref.get(value); if (ref.isInternal()) return value; + ref.setFlags(Ref.mergeFlags(ref.getFlags(), Ref.INTERNAL)); return value; } diff --git a/convex-core/src/main/java/convex/core/data/Ref.java b/convex-core/src/main/java/convex/core/data/Ref.java index b12c31379..0911a4e63 100644 --- a/convex-core/src/main/java/convex/core/data/Ref.java +++ b/convex-core/src/main/java/convex/core/data/Ref.java @@ -201,7 +201,7 @@ public int getFlags() { } /** - * Updates the Ref has the given status, at minimum + * Return a Ref that has the given status, at minimum. If status was updated, returns a new Ref * * Assumes any necessary changes to storage will be made separately. * SECURITY: Dangerous if misused since may invalidate storage assumptions @@ -220,7 +220,7 @@ public Ref withMinimumStatus(int newStatus) { } /** - * Create a new Ref of the same type with updated flags + * Return a a similar Ref of the same type with updated flags. Creates a new Ref if lags have changed. * @param newFlags New flags to set * @return Updated Ref */ diff --git a/convex-core/src/main/java/convex/core/data/Sets.java b/convex-core/src/main/java/convex/core/data/Sets.java index 54c185163..8cc20762e 100644 --- a/convex-core/src/main/java/convex/core/data/Sets.java +++ b/convex-core/src/main/java/convex/core/data/Sets.java @@ -12,12 +12,7 @@ public class Sets { static final Ref[] EMPTY_ENTRIES = new Ref[0]; @SuppressWarnings({ "rawtypes", "unchecked" }) - static final SetLeaf EMPTY = new SetLeaf(EMPTY_ENTRIES); - - static { - // Set empty Ref flags as internal embedded constant - EMPTY.getRef().setFlags(Ref.INTERNAL_FLAGS); - } + static final SetLeaf EMPTY = Cells.intern(new SetLeaf(EMPTY_ENTRIES)); @SuppressWarnings("rawtypes") public static final Ref EMPTY_REF = EMPTY.getRef(); diff --git a/convex-core/src/main/java/convex/core/data/StringShort.java b/convex-core/src/main/java/convex/core/data/StringShort.java index ec60640ab..c9f7b9ac2 100644 --- a/convex-core/src/main/java/convex/core/data/StringShort.java +++ b/convex-core/src/main/java/convex/core/data/StringShort.java @@ -34,7 +34,7 @@ public final class StringShort extends AString { /** * The canonical empty String */ - public static final StringShort EMPTY = new StringShort(Blob.EMPTY); + public static final StringShort EMPTY = Cells.intern(new StringShort(Blob.EMPTY)); protected StringShort(Blob data) { super(data.count); diff --git a/convex-core/src/main/java/convex/core/data/Strings.java b/convex-core/src/main/java/convex/core/data/Strings.java index 8f437e857..1aa387897 100644 --- a/convex-core/src/main/java/convex/core/data/Strings.java +++ b/convex-core/src/main/java/convex/core/data/Strings.java @@ -109,6 +109,10 @@ public static AString create(String s) { return Strings.create(utfBlob); } + public static T intern(T value) { + return Cells.intern(value); + } + public static AString create(CVMChar c) { return create(c.toUTFBlob()); } diff --git a/convex-core/src/main/java/convex/core/data/VectorArray.java b/convex-core/src/main/java/convex/core/data/VectorArray.java index a40929cb6..e89d2d267 100644 --- a/convex-core/src/main/java/convex/core/data/VectorArray.java +++ b/convex-core/src/main/java/convex/core/data/VectorArray.java @@ -23,7 +23,7 @@ public class VectorArray extends ASpecialVector { private ACell[] data; private int start; - + private VectorArray(ACell[] data, long start, long count) { super(count); this.data=data; diff --git a/convex-core/src/main/java/convex/core/data/VectorLeaf.java b/convex-core/src/main/java/convex/core/data/VectorLeaf.java index 2a09fea1e..9972e90be 100644 --- a/convex-core/src/main/java/convex/core/data/VectorLeaf.java +++ b/convex-core/src/main/java/convex/core/data/VectorLeaf.java @@ -80,7 +80,7 @@ public static VectorLeaf create(ACell[] elements, int offse } /** - * Creates a VectorLeaf with the given items appended to the specified tail + * Creates a VectorLeaf with the given items appended to the specified prefix vector * * @param elements Elements to add * @param offset Offset into element array diff --git a/convex-core/src/main/java/convex/etch/Etch.java b/convex-core/src/main/java/convex/etch/Etch.java index d51d99a56..2deabe5f5 100644 --- a/convex-core/src/main/java/convex/etch/Etch.java +++ b/convex-core/src/main/java/convex/etch/Etch.java @@ -324,11 +324,11 @@ private synchronized MappedByteBuffer createBuffer(int regionIndex) throws IOExc * @return Ref after writing to store * @throws IOException If an IO error occurs */ - public synchronized Ref write(AArrayBlob key, Ref value) throws IOException { + public synchronized Ref write(AArrayBlob key, Ref value) throws IOException { return write(key,0,value,INDEX_START); } - private Ref write(AArrayBlob key, int level, Ref ref, long indexPosition) throws IOException { + private Ref write(AArrayBlob key, int level, Ref ref, long indexPosition) throws IOException { if (level>=MAX_LEVEL) { throw new Error("Max Level exceeded for key: "+key); } @@ -811,7 +811,7 @@ public long readSlot(long indexPosition, int digit) throws IOException { * @return * @throws IOException */ - private Ref writeNewData(long indexPosition, int digit, AArrayBlob key, Ref value, long type) throws IOException { + private Ref writeNewData(long indexPosition, int digit, AArrayBlob key, Ref value, long type) throws IOException { long newDataPointer=appendData(key,value)|type; writeSlot(indexPosition, digit, newDataPointer); return value; @@ -824,8 +824,8 @@ private Ref writeNewData(long indexPosition, int digit, AArrayBlob key, R * @return * @throws IOException */ - private Ref updateInPlace(long position, Ref ref) throws IOException { - // ensure we have a raw poistion + private Ref updateInPlace(long position, Ref ref) throws IOException { + // ensure we have a raw position position=rawPointer(position); // Seek to status location @@ -1020,7 +1020,7 @@ private long appendNewIndexBlock(int level) throws IOException { * @return The position of the new data block * @throws IOException */ - private long appendData(AArrayBlob key,Ref ref) throws IOException { + private long appendData(AArrayBlob key,Ref ref) throws IOException { assert(key.count()==KEY_SIZE); Counters.etchWrite++; diff --git a/convex-core/src/main/java/convex/etch/EtchStore.java b/convex-core/src/main/java/convex/etch/EtchStore.java index 79d5ad5fd..d3bf8dbc8 100644 --- a/convex-core/src/main/java/convex/etch/EtchStore.java +++ b/convex-core/src/main/java/convex/etch/EtchStore.java @@ -182,6 +182,7 @@ public Ref storeRef(Ref ref, int requiredStatus, Consume try { return storeRef((Ref) r, requiredStatus, noveltyHandler, false); } catch (IOException e) { + // OK because overall function throws IOException throw Utils.sneakyThrow(e); } }; @@ -192,10 +193,8 @@ public Ref storeRef(Ref ref, int requiredStatus, Consume // perhaps need to update Ref if (cell != newObject) { - ref = ref.withValue((T) newObject); cell = newObject; - cell.attachRef(ref); // make sure we are using current ref within new cell } } @@ -209,33 +208,29 @@ public Ref storeRef(Ref ref, int requiredStatus, Consume + " ref of class " + Utils.getClassName(cell) + " with store " + this); } - Ref result; - // ensure status is set when we write to store ref = ref.withMinimumStatus(requiredStatus); - cell.attachRef(ref); // make sure we are using current ref within cell - result = etch.write(fHash, (Ref) ref); + ref = etch.write(fHash, ref); if (!embedded) { - // Ensure we have soft Refpointing to this store - result = result.toSoft(this); + // Ensure we have soft Ref pointing to this store + ref = ref.toSoft(this); } - cell.attachRef(result); - addToCache(result); // cache for subsequent writes + cell.attachRef(ref); // make sure we are using current ref within cell + addToCache(ref); // cache for subsequent writes // call novelty handler if newly persisted non-embedded if (noveltyHandler != null) { if (!embedded) - noveltyHandler.accept(result); + noveltyHandler.accept((Ref) ref); } - return (Ref) result; } else { // no need to write, just tag updated status ref = ref.withMinimumStatus(requiredStatus); - cell.attachRef(ref); - return ref; } + cell.attachRef(ref); + return ref; } protected void addToCache(Ref ref) { diff --git a/convex-core/src/test/java/convex/core/data/BlocksTest.java b/convex-core/src/test/java/convex/core/data/BlocksTest.java index 3ebd307a0..3c3dea92c 100644 --- a/convex-core/src/test/java/convex/core/data/BlocksTest.java +++ b/convex-core/src/test/java/convex/core/data/BlocksTest.java @@ -28,6 +28,13 @@ public void testEquality() throws BadFormatException { RecordTest.doRecordTests(b1); } + @Test public void testValues() { + long ts = System.currentTimeMillis(); + Block b1 = Block.create(ts, Vectors.empty()); + + assertEquals(2,b1.values().count()); + } + @Test public void testTransactions() throws BadSignatureException { AKeyPair kp = InitTest.HERO_KEYPAIR; diff --git a/convex-core/src/test/java/convex/core/data/CollectionsTest.java b/convex-core/src/test/java/convex/core/data/CollectionsTest.java index d6f7c8674..11a6602e3 100644 --- a/convex-core/src/test/java/convex/core/data/CollectionsTest.java +++ b/convex-core/src/test/java/convex/core/data/CollectionsTest.java @@ -87,6 +87,9 @@ public static void doCountableTests(ACountable a) { assertTrue((a instanceof Keyword)||(a instanceof Symbol)||(a instanceof Address)); } else { assertEquals(0,empty.count()); + + // Canonical version of any empty structure should be internal + RefTest.checkInternal(empty.toCanonical()); } if (n == 0) { diff --git a/convex-core/src/test/java/convex/core/data/RefTest.java b/convex-core/src/test/java/convex/core/data/RefTest.java index 8ba8471e1..fdb8269d4 100644 --- a/convex-core/src/test/java/convex/core/data/RefTest.java +++ b/convex-core/src/test/java/convex/core/data/RefTest.java @@ -25,6 +25,7 @@ import convex.core.lang.Core; import convex.core.lang.RT; import convex.core.lang.Symbols; +import convex.core.util.Utils; import convex.test.Samples; public class RefTest { @@ -356,6 +357,7 @@ public void testInternal() { // Empty data structures checkInternal(Maps.empty()); + checkInternal(Sets.empty()); checkInternal(Vectors.empty()); checkInternal(Lists.empty()); checkInternal(Index.EMPTY); @@ -371,7 +373,7 @@ public void testInternal() { public static void checkInternal(T a) { Ref ref=Ref.get(a); - assertTrue(ref.isInternal()); + assertTrue(ref.isInternal(),()->"Not internal ref: "+a+" of type "+Utils.getClass(a)); assertSame(a,ref.getValue()); try { diff --git a/convex-core/src/test/java/convex/core/data/VectorsTest.java b/convex-core/src/test/java/convex/core/data/VectorsTest.java index 19166ffd1..eb44569b5 100644 --- a/convex-core/src/test/java/convex/core/data/VectorsTest.java +++ b/convex-core/src/test/java/convex/core/data/VectorsTest.java @@ -31,13 +31,14 @@ public class VectorsTest { @Test public void testEmptyVector() { - AVector lv = Vectors.empty(); - AArrayBlob d = lv.getEncoding(); + AVector e = Vectors.empty(); + RefTest.checkInternal(e); + AArrayBlob d = e.getEncoding(); assertArrayEquals(new byte[] { Tag.VECTOR, 0 }, d.getBytes()); - - assertSame(lv,Vectors.empty()); - - assertSame(lv,Vectors.wrap(Cells.EMPTY_ARRAY)); + assertSame(e,Vectors.empty()); + assertSame(e,Vectors.wrap(Cells.EMPTY_ARRAY)); + assertSame(e,Vectors.create(new ACell[0])); + assertSame(e,Vectors.create(new ACell[0],0,0)); } @Test