Implement property presence bitset per layer - #2658
Open
arienandalibi wants to merge 58 commits into
Open
Conversation
…global property id i corresponding to the segment. Persisted to disk. In-memory part grows incrementally.
…ow) of properties present in each (segment, layer) to efficiently retrieve the lists of relevant properties. We union over all segments in the storage. We also filter out props/metadata that aren't present now instead of all globally registered properties.
… for properties and one for metadata in Meta. Wired it in so that layers where we know the property isn't present are skipped.
…und in this layer. Added these for the node storage and edge storage. Skip layers in tprop_iter_layers, which is used in time semantics.
…es. These were used to track property presence per-(segment, layer) on disk and in-memory. Now, we only keep track of property presence per layer. We skip layers where we know the property was added, but that's it.
… previous layer property schemas (property presence bitsets) to avoid work. All of it was redundant and didn't save anything. Removed functions that pass these `LayerPropSchema`s around.
…pe, dst_node_type). Introduce limits for max number of edges/values in properties and edges.
# Conflicts: # raphtory-storage/src/graph/nodes/node_storage_ops.rs # raphtory/src/db/graph/views/filter/model/degree_filter.rs
…g all properties. As soon as a property key has more than ENUM_BOUNDARY values, we skip property value collection.
…ies and metadata. This is held by GraphWithVectors(Inner). It gets passed to GqlGraph for base/unfiltered (e.g. materialized) graphs, which passes them to EdgeSchema by Arc.
…String, String) type alias. This allows us to pass references to the key easily. Faster on cache lookup
# Conflicts: # raphtory-storage/src/graph/nodes/node_storage_ops.rs
# Conflicts: # raphtory-graphql/src/data.rs # raphtory-graphql/src/graph.rs # raphtory-storage/src/graph/nodes/node_storage_ops.rs
…es and metadata per layer. We use the maintained property presence bitsets to do this
… rid of NodeSchemaKey. Wire the cache through in the schemas.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
⚠️ Performance Alert ⚠️
Possible performance regression was detected for benchmark 'Rust Benchmark'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 2.
| Benchmark suite | Current: 9ff2eba | Previous: 9823ef7 | Ratio |
|---|---|---|---|
large/1k random edge additions |
1554006 ns/iter (± 117540) |
658628 ns/iter (± 51943) |
2.36 |
large/1k random edge additions with numeric string input |
2061410 ns/iter (± 146492) |
876236 ns/iter (± 86855) |
2.35 |
lotr_graph/num_edges |
5 ns/iter (± 0) |
0 ns/iter (± 0) |
+∞ |
lotr_graph/num_nodes |
5 ns/iter (± 0) |
1 ns/iter (± 0) |
5 |
lotr_graph/graph_latest |
2 ns/iter (± 0) |
0 ns/iter (± 0) |
+∞ |
lotr_graph_materialise/materialize |
7944887 ns/iter (± 47902) |
1564816 ns/iter (± 35303) |
5.08 |
lotr_graph_window_100/num_nodes |
13 ns/iter (± 0) |
5 ns/iter (± 0) |
2.60 |
lotr_graph_window_100_materialise/materialize |
7834944 ns/iter (± 41746) |
1669150 ns/iter (± 10700) |
4.69 |
lotr_graph_window_10/has_node_existing |
126 ns/iter (± 9) |
62 ns/iter (± 11) |
2.03 |
lotr_graph_window_10_materialise/materialize |
3170133 ns/iter (± 23555) |
971980 ns/iter (± 4278) |
3.26 |
lotr_graph_subgraph_10pc/num_nodes |
12 ns/iter (± 0) |
4 ns/iter (± 0) |
3 |
lotr_graph_subgraph_10pc_materialise/materialize |
1986256 ns/iter (± 33323) |
334634 ns/iter (± 1287) |
5.94 |
lotr_graph_subgraph_10pc_windowed/has_node_existing |
127 ns/iter (± 9) |
62 ns/iter (± 14) |
2.05 |
lotr_graph_subgraph_10pc_windowed_materialise/materialize |
1117560 ns/iter (± 6569) |
230399 ns/iter (± 2617) |
4.85 |
lotr_graph_window_50_layered/has_node_existing |
365 ns/iter (± 21) |
129 ns/iter (± 12) |
2.83 |
lotr_graph_window_50_layered/graph_latest |
89083 ns/iter (± 3439) |
36649 ns/iter (± 916) |
2.43 |
lotr_graph_window_50_layered_materialise/materialize |
27743104 ns/iter (± 118789) |
3488825 ns/iter (± 24948) |
7.95 |
lotr_graph_persistent_window_50_layered/num_edges_temporal |
618211 ns/iter (± 5206) |
192686 ns/iter (± 1569) |
3.21 |
lotr_graph_persistent_window_50_layered/has_node_existing |
382 ns/iter (± 312) |
174 ns/iter (± 83) |
2.20 |
lotr_graph_persistent_window_50_layered/graph_latest |
125403 ns/iter (± 1419) |
57549 ns/iter (± 4809) |
2.18 |
lotr_graph_persistent_window_50_layered_materialise/materialize |
46053339 ns/iter (± 155965) |
5298035 ns/iter (± 147912) |
8.69 |
This comment was automatically generated by workflow using github-action-benchmark.
…e bitset to integration-tests (in pometry-storage) instead of raphtory-tests (in Raphtory). Fixed queries and responses to match new schemas.
arienandalibi
marked this pull request as ready for review
July 8, 2026 08:15
fabubaker
self-requested a review
July 8, 2026 16:08
fabianmurariu
requested changes
Jul 27, 2026
fabianmurariu
left a comment
fabianmurariu
left a comment
Collaborator
There was a problem hiding this comment.
Maybe we can avoid some clones?
| self.graph_stats.update_time(t.t()); | ||
| // Update the per-layer property presence bitset in Meta. | ||
| // `.inspect` runs once per emitted item as the iterator is consumed in `insert_edge_internal` | ||
| let meta = self.writer.edge_meta().clone(); |
Collaborator
There was a problem hiding this comment.
This is going to light up in bulk loading quite badly, we need a clone free variant
| self.layer_prop_presence | ||
| .read_recursive() | ||
| .get(layer_id.0) | ||
| .and_then(|row| row.get(prop_id)) |
| key.to_string(), | ||
| mapper | ||
| .get_dtype(id) | ||
| .expect("type for internal id should always exist") |
Collaborator
There was a problem hiding this comment.
unwrap_or_else with a panic inside is a bit better
# Conflicts: # db4-storage/src/pages/node_page/writer.rs
…rty presence bitsets. Instead, we add a function on SegmentContainer which can do it with borrowing while avoiding borrowing issues (mut and immut).
… a RwLock) so that we only try to mark props in Meta's bitsets when the container first sees the prop. Avoids lock contention.
fabubaker
changed the title
… path. We don't always check to update on a per-property basis anymore. During bulk ingestion, we try to do the marking once per chunk. Currently over-marking the properties in a layer.
…ssue. Now, we only set exact (layer, prop) pairs in the bitset.
# Conflicts: # raphtory-graphql/src/data.rs # raphtory-graphql/src/model/schema/edge_schema.rs # raphtory-graphql/src/model/schema/layer_schema.rs # raphtory-graphql/src/model/schema/mod.rs # raphtory-graphql/src/model/schema/node_schema.rs # raphtory-graphql/src/model/schema/property_schema.rs
…et is copied from the source graph into the materialized graph. Since some layers/properties can be filtered out, we have to make sure the bitset is recreated to reflect this
…this PR. Mainly, we're getting rid of EdgeSchema and replacing it for properties/metadata on LayerSchema directly.
…ime of associated DiskNodeRef/DiskEdgeRef. 2) Split PropMapper's write_locked() into write_locked_mappers() and write_locked_layer_presence(). Many times, we don't need to lock layer presence. 3) Update collect_variants to not Hash on (String, PropType) as key. PropType used to do a memory allocation and sort on PropType::Map. Now, we collect into a vec indexed by prop_id. 4) Graph prop insertion now marks the bitset properly
# Conflicts: # db4-storage/src/pages/edge_page/bulk_writer.rs # db4-storage/src/pages/node_page/writer.rs # raphtory-graphql/src/data.rs # raphtory-graphql/src/model/graph/graph.rs # raphtory-graphql/src/python/client/remote_schema.rs
2) Get rid of double filtering using the bitset on filtered_edge_metadata 3) Re-use collect variants for the node schema so we collect all variants in one pass rather than iterate per property key per node.
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
What changes were proposed in this pull request?
Adding a bitset (Vec) to keep track of which properties exist in which layers. Use this to optimize iterators (especially relevant when using disk storage). Also optimized the generation of schemas (node schema, edge schema, ...) by iterating edges only once and implementing a cache for already generated schemas.
Why are the changes needed?
Optimizations can be made to improve UI experience and disk accesses.
Does this PR introduce any user-facing change? If yes is this documented?
It shouldn't
How was this patch tested?
Will need to be tested using the UI
Are there any further changes required?
There shouldn't be