Conversation
DataTreeWriter and DataTreeReader built an IPath for every node they visited, only to hand it to the flattener. The flatteners only ever asked whether it was the root, whose data holds the parent backpointer and must not be written. This cost a Path plus a segment array copy for every node of every tree in the delta chain, on each save and again on each startup. Pass whether the node is the tree root instead. The reader tracks the depth it needs to recognize a project node. Serializing a tree of 615,000 nodes (600 projects) now allocates 14 MB instead of 56 MB when writing and 73 MB instead of 110 MB when reading. Writing is 12% faster with C2-compiled code, and both directions are about a third faster with C1 only, which is closer to the one-time read at startup. Contributes to eclipse-platform#2887 Assisted-by: multiple AI agents and layers of automated tooling 🤖
8b9da24 to
3286899
Compare
This table data is about heap memory used during read/write, nothing changes on disc, correct? |
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Required bundle-version increments and regression coverage for project renaming are missing.
Review effort: Balanced
Findings: 1
Open (3)
What changed in this PR
Optimizes workspace tree serialization by replacing per-node path construction with root/depth tracking while preserving the on-disk format.
Changes:
- Passes root-state flags through tree serializers.
- Uses reader depth to identify project nodes.
- Updates flatteners and serialization test helpers.
| File | Description |
|---|---|
ElementTreeSerializationTestHelper.java |
Adapts test flattener signatures. |
IElementInfoFlattener.java |
Removes path parameters. |
ElementTreeWriter.java |
Uses root flags when writing. |
ElementTreeReader.java |
Uses root flags when reading. |
SaveManager.java |
Adapts resource serialization methods. |
IDataFlattener.java |
Replaces paths with root flags. |
DataTreeWriter.java |
Avoids constructing descendant paths. |
DataTreeReader.java |
Tracks traversal depth instead of paths. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Yes. from the PR description: the on-disk format is unchanged. |
|
Test suggestion from Copilot is already covered by ProjectSnapshotTest.testLoadWithRename/testLoadWithRename2, which load a snapshot into a renamed project and assert the descendants keep their names. The version suggestion were wrong, we have our automatic version updates handled by GH actions. |
Writing and reading the workspace tree built an IPath for every node, only for the flatteners to check whether it was the root. On a large workspace that is a Path and a segment array copy per node of every tree in the delta chain, paid on each save and again on each startup. The serializers now pass a root flag instead, and the reader tracks the depth it needs to recognize project nodes, so the on-disk format is unchanged.
Serializing a tree of 615,000 nodes (600 projects), compared against master:
The C1 row is the closer match for the startup read, which runs once before the JIT has warmed up.
Contributes to #2887