Ref testing and refactoring for better persistence behaviour

convex-core/src/main/java/convex/core/data/ACell.java

@@ -439,7 +439,10 @@ public <R extends ACell> Ref<R> 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<ACell>) ref;
 	}
 

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 extends ACell> T persist(T a) throws IOException {
-		return persist(a,Stores.current());
+		AStore store=Stores.current();
+		return persist(a,store);
 	}
 
 	/**
@@ -247,14 +248,18 @@ public static void visitBranches(ACell a, Consumer<ACell> 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 <T> Type of Cell
 	 * @param value Value to intern
 	 * @return Interned Cell
 	 */
 	public static <T extends ACell> T intern(T value) {
 		Ref<T> ref=Ref.get(value);
 		if (ref.isInternal()) return value;
+
 		ref.setFlags(Ref.mergeFlags(ref.getFlags(), Ref.INTERNAL));
 		return value;
 	}

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<T> 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
 	 */

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<SetLeaf> EMPTY_REF = EMPTY.getRef();

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);

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 extends AString> T intern(T value) {
+		return Cells.intern(value);
+	}
+
 	public static AString create(CVMChar c) {
 		return create(c.toUTFBlob());
 	}

convex-core/src/main/java/convex/core/data/VectorArray.java

@@ -23,7 +23,7 @@ public class VectorArray<T extends ACell> extends ASpecialVector<T> {
 
 	private ACell[] data;
 	private int start;
-
+	
 	private VectorArray(ACell[] data, long start, long count) {
 		super(count);
 		this.data=data;

convex-core/src/main/java/convex/core/data/VectorLeaf.java

@@ -80,7 +80,7 @@ public static <T extends ACell> VectorLeaf<T> 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

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<ACell> write(AArrayBlob key, Ref<ACell> value) throws IOException {
+	public synchronized <T extends ACell > Ref<T> write(AArrayBlob key, Ref<T> value) throws IOException {
 		return write(key,0,value,INDEX_START);
 	}
 
-	private Ref<ACell> write(AArrayBlob key, int level, Ref<ACell> ref, long indexPosition) throws IOException {
+	private <T extends ACell > Ref<T> write(AArrayBlob key, int level, Ref<T> 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<ACell> writeNewData(long indexPosition, int digit, AArrayBlob key, Ref<ACell> value, long type) throws IOException {
+	private <T extends ACell > Ref<T> writeNewData(long indexPosition, int digit, AArrayBlob key, Ref<T> value, long type) throws IOException {
 		long newDataPointer=appendData(key,value)|type;
 		writeSlot(indexPosition, digit, newDataPointer);
 		return value;
@@ -824,8 +824,8 @@ private Ref<ACell> writeNewData(long indexPosition, int digit, AArrayBlob key, R
      * @return
      * @throws IOException
      */
-	private Ref<ACell> updateInPlace(long position, Ref<ACell> ref) throws IOException {
-		// ensure we have a raw poistion
+	private <T extends ACell > Ref<T> updateInPlace(long position, Ref<T> 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<ACell> ref) throws IOException {
+	private long appendData(AArrayBlob key,Ref<?> ref) throws IOException {
 		assert(key.count()==KEY_SIZE);
 		Counters.etchWrite++;
 

convex-core/src/main/java/convex/etch/EtchStore.java

@@ -182,6 +182,7 @@ public <T extends ACell> Ref<T> storeRef(Ref<T> ref, int requiredStatus, Consume
 				try {
 					return storeRef((Ref<ACell>) r, requiredStatus, noveltyHandler, false);
 				} catch (IOException e) {
+					// OK because overall function throws IOException
 					throw Utils.sneakyThrow(e);
 				}
 			};
@@ -192,10 +193,8 @@ public <T extends ACell> Ref<T> storeRef(Ref<T> 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 <T extends ACell> Ref<T> storeRef(Ref<T> ref, int requiredStatus, Consume
 						+ " ref of class " + Utils.getClassName(cell) + " with store " + this);
 			}
 
-			Ref<ACell> 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<ACell>) 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<ACell>) ref);
 			}
-			return (Ref<T>) 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 <T extends ACell> void addToCache(Ref<T> ref) {

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;

convex-core/src/test/java/convex/core/data/CollectionsTest.java

@@ -87,6 +87,9 @@ public static <T extends ACell> void doCountableTests(ACountable<T> 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) {

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 <T extends ACell> void  checkInternal(T a)  {
 		Ref<T> ref=Ref.get(a);
-		assertTrue(ref.isInternal());
+		assertTrue(ref.isInternal(),()->"Not internal ref: "+a+" of type "+Utils.getClass(a));
 		assertSame(a,ref.getValue());
 
 		try {

convex-core/src/test/java/convex/core/data/VectorsTest.java

@@ -31,13 +31,14 @@ public class VectorsTest {
 
 	@Test
 	public void testEmptyVector() {
-		AVector<AString> lv = Vectors.empty();
-		AArrayBlob d = lv.getEncoding();
+		AVector<AString> 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