Significant performance issue when adding triples to a Dataset by a Graph object

[mgberg]

I was trying out pyLODE's supermodel profile and noticed it was extremely slow. After some debugging, I found the issue which can be demonstrated by this script:

from time import perf_counter

from rdflib import Dataset, Graph

file = "https://www.w3.org/ns/shacl.ttl"

source_data = Graph().parse(file)

test_graph = Graph()
start = perf_counter()
for s, p, o in source_data:
    test_graph.add((s, p, o))
base_dur = perf_counter() - start
print(f"Added {len(test_graph)} triples to graph in {base_dur:.3f} seconds")

test_ds = Dataset()
start = perf_counter()
for s, p, o in source_data:
    test_ds.add((s, p, o, source_data.identifier))
dur = perf_counter() - start
print(f"Added {len(test_graph)} triples to dataset by identifier in {dur:.3f} seconds ({dur / base_dur:.2f}x the first case)")
assert source_data in test_ds.graphs()

test_ds = Dataset()
start = perf_counter()
for s, p, o in source_data:
    test_ds.add((s, p, o, source_data))
dur = perf_counter() - start
print(f"Added {len(test_graph)} triples to dataset by graph in {dur:.3f} seconds ({dur / base_dur:.2f}x the first case)")
assert source_data in test_ds.graphs()

I get the following output from this script:

Added 1128 triples to graph in 0.012 seconds
Added 1128 triples to dataset by identifier in 0.017 seconds (1.37x the first case)
Added 1128 triples to dataset by graph in 10.057 seconds (827.36x the first case)

When Dataset.add is called where the context is a Graph, it takes orders of magnitude longer to add the triples to the dataset.

pyLODE was so slow because it did that third case here: https://github.com/RDFLib/pyLODE/blob/eb7a1d680ae08df1da13d14e9ecea8b7a7b670e8/pylode/profiles/supermodel/dataset.py#L27

This slowdown was introduced at this line and commit:

rdflib/rdflib/graph.py

Line 2296 in 1b77686

_graph.__iadd__(c)

Whenever Dataset.add (really ConjunctiveGraph.add) is called where the context is a Graph, before the triple gets added to the dataset, the entire content of that context graph is getting copied into the graph created by ConjunctiveGraph._graph, which is clearly unnecessary in this context. I'm not exactly sure why that above change was made.

Also note that the type annotation of ConjunctiveGraph.add is _TripleOrOptionalQuadType, which types the context element of the quad as Optional[_ContextType], and _ContextType is equivalent to Graph. However, whenever I use Dataset.add I always use an IdentifiedNode which doesn't suffer from this issue and works as expected (as shown in the above example).

Given that many of the functions mentioned here aren't fully documented and that the type annotations are potentially not quite right, I'm not quite sure what should be considered the actual issue. Presumably it is at least one of the following:

• The type annotation of ConjunctiveGraph.add is incorrect and pyLODE should be loading the Dataset differently (i.e. pyLODE should not be providing a Graph as the context, but rather the Graph.identifier)
• ConjunctiveGraph._graph should not be called from ConjunctiveGraph.add (via ConjunctiveGraph._spoc)
• ConjunctiveGraph._graph should not be copying the full contents of the provided c

I'd be curious to know what others think about this and if/what changes are necessary.

[edmondchuc]

Hi @mgberg, thanks for describing the issue in such great detail, the added context really helps.

pyLODE patched the Dataset.add_graph() (see pylode.profiles.supermodel.dataset) as a workaround for the same issue described in #3335. Now that this has been fixed in RDFLib, that patch is no longer necessary.

Prior to the fix introduced in PR #3259, the backing store could become silently inconsistent, making the issue difficult to detect. The fix removes this ambiguity by trading computation time (copying the statements) for stronger store state guarantees. Users who want to avoid copying statements when calling add_graph() should follow the pattern described in #698 (comment), where graphs are created via the dataset rather than added to it. We should add this pattern to the user docs if it's not already there.

I've opened a proposed fix in PR #3394 and would appreciate your feedback.

[mgberg]

@edmondchuc #3394 looks good to me, which makes it clear that:

• _graph is used to just coalesce to a Graph, and
• if you provide a Graph object to add_graph or graph, the contents of the provided graph should be added to the dataset.

I agree, it would be helpful to add a comment to the documentation about this if it's not already there.

Interestingly, I've always used the pattern described in #698 anyway so I've personally never encountered the store inconsistency issue before.