Provide Persistance for Persistance.Reference #9326
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
JaroslavTulach
added
CI: No changelog needed
JaroslavTulach
removed
the
CI: Keep up to date
2 tasks
JaroslavTulach
commented
Mar 8, 2024
| @@ -470,4 +501,11 @@ public record ServiceSupply(Service supply) {} | |||
|
|
|||
| @Persistable(clazz = IdHolder.class, id = 432876) | |||
| public record IdHolder(UUID id) {} | |||
|
|
|||
| @Persistable(clazz = RefHolder.class, id = 436872) | |||
| public record RefHolder(Persistance.Reference<UUID> id) { | |||
There was a problem hiding this comment.
Have a class with field Persistance.Reference and write/generate Persistance instance for it. In this case we generate the class with @Persistable(clazz = RefHolder.class... annotation.
JaroslavTulach
commented
Mar 8, 2024
| public void refHolderWithUUID() throws Exception { | ||
| var id = UUID.randomUUID(); | ||
| var il = new RefHolder(id); | ||
| var in = serde(RefHolder.class, il, -1, null); |
There was a problem hiding this comment.
This stores an instance of RefHolder and reads another instace of RefHolder back from the array. Only then when one in.id().get(UUID.class) the instance of UUID gets deserialized.
hubertp
approved these changes
Mar 8, 2024
|
As discussed, here's the repro for the problem with cycles in serialization that I'm encountering:
After some digging and relying on the cycle detector, it our initial hypothesis ( which is quite expected since it is indeed the
|
|
It seems that I've got a (partial) fix for the issue. It seems that writing is fixed by ensuring that the currently written object is registered before serializing it (so that once we encounter the cycle during serialization, we already see the backreference and avoid the infinite loop). Seems to be taken care of by this patch: diff --git a/lib/java/persistance/src/main/java/org/enso/persist/PerGenerator.java b/lib/java/persistance/src/main/java/org/enso/persist/PerGenerator.java
--- a/lib/java/persistance/src/main/java/org/enso/persist/PerGenerator.java (revision c1fead2ffa276279c28447db7e17118cc92cb2ed)
+++ b/lib/java/persistance/src/main/java/org/enso/persist/PerGenerator.java (revision 967cffa4b916e42f194e565f4e664fb7d293c1cd)
@@ -59,14 +59,15 @@
java.lang.Object obj = writeReplace.apply(t);
java.lang.Integer found = knownObjects.get(obj);
if (found == null) {
+ found = this.position;
+ knownObjects.put(obj, found);
+
org.enso.persist.Persistance<?> p = map.forType(obj.getClass());
java.io.ByteArrayOutputStream os = new ByteArrayOutputStream();
p.writeInline(obj, new ReferenceOutput(this, os));
- found = this.position;
byte[] arr = os.toByteArray();
main.write(arr);
this.position += arr.length;
- knownObjects.put(obj, found);
}
return found;
}
@@ -83,18 +84,18 @@
}
java.lang.Integer found = knownObjects.get(obj);
if (found == null) {
+ found = position;
+ knownObjects.put(obj, found);
var os = new ByteArrayOutputStream();
var osData = new ReferenceOutput(this, os);
p.writeInline(obj, osData);
- found = position;
if (os.size() == 0) {
os.write(0);
}
byte[] arr = os.toByteArray();
main.write(arr);
position += arr.length;
- knownObjects.put(obj, found);
if (histogram != null) {
histogram.register(obj.getClass(), arr.length);
}After this patch I was able to rebuild indices. I'm slightly worried that there may be some issues when reading this back, since I'm getting errors like: But I'm not 100% sure if this is it or just unrelated errors? More problems after clean build: |
I have a feeling it fails a lot of |
|
Jaroslav Tulach reports a new STANDUP for yesterday (2024-03-08): Progress: -
Next Day: Back to work on Monday |
|
Request to also support "cycles" in the graph is |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Pull Request Description
Provide default
Persistanceimplementation forPersistance.Referenceto allow us to persist fields of that type easily. The reference is used to load object lazily. When constructing the objects in memory one usesReference.of(anObject), stores it in a field and after deserializing one gets an instance of theReferenceback (but without the original object being deserialized). Later one callsref.get(TypeOfTheObject.class)to deserialize also instance of the original object.Checklist
Please ensure that the following checklist has been satisfied before submitting the PR:
Java,
style guides.