perf: cache component parse/compile - #14905
Draft
jordanrfrazier wants to merge 2 commits into
Draft
Conversation
…parsing per vertex eval_custom_component_code AST-parsed the source twice, compiled it, and exec'd it on every call -- for every vertex of every flow build. Independent profiling puts this at 57-86% of all Graph.from_payload time across six real flows. Now the immutable output is cached (class name plus two code objects) and a FRESH class is exec'd from it on each call. Caching the CLASS instead was tried and REVERTED: byte-identical source from two tenants would share one class object, so any class-level mutable state -- the classic `_seen = []` mistake, and Langflow targets non-expert Python users -- crosses between them. It does not even need two tenants: sibling vertices in one graph layer build concurrently via asyncio.gather. exec()-ing a class statement's code object always builds a new class, so caching the code object is safe where caching the class is not. exec_globals is rebuilt per call for the same reason. MEASURED on real starter flows (Graph.from_payload, before -> after): Vector Store RAG 7 nodes 45.44 ms -> 7.42 ms -83.7% Hybrid Search RAG 11 nodes 54.34 ms -> 13.19 ms -75.7% Document Q&A 7 nodes 42.52 ms -> 7.68 ms -81.9% Simple Agent 6 nodes 33.41 ms -> 6.79 ms -79.7% Basic Prompting 6 nodes 9.49 ms -> 3.37 ms -64.5% Per component, over 23 real components: median 1.589 ms -> 0.100 ms. The win is largest where components are heavy (Knowledge 12.99 -> 0.658 ms, File 9.32 -> 0.397 ms), which is why a small synthetic flow badly understates it -- a 2-node benchmark shows only -14% CPU. The residual 0.1 ms per component is the fresh exec, and it is the price of the isolation: import resolution accounts for only 0.038-0.063 ms of it, so there is no meaningful further win available without sharing state between instances. Includes the GuardrailsComponent fix, which is NOT optional here: the cache is only safe if no shipped component writes to module-level state, and test_no_shipped_component_writes_to_module_level_state fails without it. That coupling is why the two ship together. component_index.json carries the regenerated code_hash for that component. Tests: 209 pass. Verified by mutation -- reintroducing the class cache produces 11 failures, including cross-tenant evidence such as "thread 23 saw ['user-A-secret', 'user-B-secret', 'thread-23']". The cache stores code objects, never ASTs: 18.4 KB per entry, 9.4 MB at maxsize=512 (an AST-retaining cache would be ~260 MB). test_vertex_with_nothing_to_load_does_not_touch_the_connection_pool is deliberately NOT included: it guards the early return in update_params_with_load_from_db_fields and belongs with a loading.py change.
Contributor
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueThanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
`definitions` collected every module-level statement including the component's own `ast.ClassDef`, so `definitions_code_obj` already contained the class. `_instantiate_component_class` exec'd it (class created), then `build_class_constructor` exec'd `class_code_obj` (class created again, the first discarded). Verified with a `__build_class__` counter: the class was built twice per `_instantiate_component_class` call, now once. Excluding it from `definitions`: chat 110.2 us -> 81.0 us (-27%) chat_output 119.9 us -> 85.4 us (-29%) and stops caching two code objects where one suffices. Module-level helpers, constants and sidecar classes are still compiled into `definitions_code_obj` — only the component class itself is skipped. Verified with a component using a module constant, a helper function and a second class: output identical to the untouched `create_class` reference path. Corpus check: 150 shipped components build identically on the cached and reference paths, 0 disagreements. Suites: 209 lfx custom, 53 backend (white-box validate + multi-tenant isolation).
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
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.
perf(components): cache component parse/compile output instead of re-parsing per vertex
The problem
eval_custom_component_codeparsed the source twice withast.parse, compiledit, and
exec'd it — once per vertex, on every flow build. The source is aconstant: for a shipped component it is byte-identical on every request, forever.
Independent profiling puts this at 57–86% of all
Graph.from_payloadtime,measured across six real flows.
The fix
Cache the immutable output — the class name and two code objects — and
execafresh class from it on each call.
Why not just cache the class?
That was tried, and reverted: it is a cross-tenant data leak.
Two tenants with byte-identical source would share one class object, and with it
any class-level mutable state — the classic
_seen = []mistake, which mattersbecause Langflow targets non-expert Python users. It does not even take two
tenants: sibling vertices in one graph layer build concurrently via
asyncio.gather.exec()-ing a class statement's code object always produces a new class, socaching the code object is safe where caching the class is not.
exec_globalsis rebuilt per call for the same reason.
Measured
Graph.from_payloadon real starter flows:Per component, over 23 real components: median 1.589 ms → 0.100 ms.
The win is largest where components are heavy (Knowledge 12.99 → 0.658 ms, File
9.32 → 0.397 ms) — which is why a small synthetic flow badly understates it. A
2-node benchmark shows only −14% CPU.
What is left, and why it stays
The residual ~0.1 ms per component is the fresh
exec. That is the price of theisolation, and it is not worth chasing: import resolution accounts for only
0.038–0.063 ms of it, so there is no meaningful further win without sharing
state between instances.
Why the Guardrails fix ships with this
It is not incidental. The cache is only safe if no shipped component writes to
module-level state, and
test_no_shipped_component_writes_to_module_level_statefails without the fix.
component_index.jsoncarries that component'sregenerated
code_hash.Verification
209 tests pass.
Mutation-verified: reintroducing the class cache produces 11 failures, with
direct cross-tenant evidence —
Memory: the cache stores code objects, never ASTs — 18.4 KB per entry, 9.4 MB at
maxsize=512. An AST-retaining cache would be roughly 260 MB.Deliberately not included
test_vertex_with_nothing_to_load_does_not_touch_the_connection_poolguards theearly return in
update_params_with_load_from_db_fields. It belongs with aloading.pychange, not this one.