Skip to content

Commit

Permalink
Fix the perf issue in building nodes from splits. (#10766)
Browse files Browse the repository at this point in the history
* Fix the perf issue in building nodes from splits.

Create the `relationships` object only once. Otherwise, it recomputes the whole text's hash for every node. It is very inefficient for long text.

An alternative approach would be to cache the hash property. However, it wasn't so straightforward as `Document` isn't a cacheable type. I also do not know Python very well, maybe it would be enough to store a simple null and if it isn't null, then don't recompute? However, the most important reason is I'm not sure about the side effects and the existing assumption that the node is mutable and the hash always reflects the state during the call (unless we modify the object in multiple threads). This change doesn't break any assumptions. If the document was modified while we were creating nodes extracted from it, something would be very wrong.

Benchmarks taken on a document attached to the bug:

Before: Execution time for build_nodes_from_splits: 53.69 seconds

After: Execution time for build_nodes_from_splits: 0.18 seconds

* Fix the formatting
  • Loading branch information
preemoDez authored Feb 15, 2024
1 parent b5e96d4 commit f2d9472
Showing 1 changed file with 7 additions and 3 deletions.
10 changes: 7 additions & 3 deletions llama-index-core/llama_index/core/node_parser/node_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,10 @@ def build_nodes_from_splits(
ref_doc = ref_doc or document
id_func = id_func or default_id_func
nodes: List[TextNode] = []
"""Calling as_related_node_info() on a document recomputes the hash for the whole text and metadata"""
"""It is not that bad, when creating relationships between the nodes, but is terrible when adding a relationship"""
"""between the node and a document, hence we create the relationship only once here and pass it to the nodes"""
relationships = {NodeRelationship.SOURCE: ref_doc.as_related_node_info()}
for i, text_chunk in enumerate(text_splits):
logger.debug(f"> Adding chunk: {truncate_text(text_chunk, 50)}")

Expand All @@ -54,7 +58,7 @@ def build_nodes_from_splits(
metadata_seperator=document.metadata_seperator,
metadata_template=document.metadata_template,
text_template=document.text_template,
relationships={NodeRelationship.SOURCE: ref_doc.as_related_node_info()},
relationships=relationships,
)
nodes.append(image_node) # type: ignore
elif isinstance(document, Document):
Expand All @@ -67,7 +71,7 @@ def build_nodes_from_splits(
metadata_seperator=document.metadata_seperator,
metadata_template=document.metadata_template,
text_template=document.text_template,
relationships={NodeRelationship.SOURCE: ref_doc.as_related_node_info()},
relationships=relationships,
)
nodes.append(node)
elif isinstance(document, TextNode):
Expand All @@ -80,7 +84,7 @@ def build_nodes_from_splits(
metadata_seperator=document.metadata_seperator,
metadata_template=document.metadata_template,
text_template=document.text_template,
relationships={NodeRelationship.SOURCE: ref_doc.as_related_node_info()},
relationships=relationships,
)
nodes.append(node)
else:
Expand Down

0 comments on commit f2d9472

Please sign in to comment.