Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cache the result of the compile_expr function #1528

Merged
merged 2 commits into from
Sep 16, 2021

Conversation

mdickinson
Copy link
Member

@mdickinson mdickinson commented Sep 15, 2021

This PR adds an LRU cache for the compile_expr function, to eliminate unnecessary calls to _as_graph. I also made a one-line change in the observe function so that all expression compilation goes through compile_expr.

I've also boosted the size of the three parsing-related caches from 128 entries to 256.

I opted not to do a per-Expression-object caching of the result of the _as_graphs call (as suggested in the #1516 discussion); it adds extra complication in that it makes the Expression objects mutable, and so introduces potential thread-safety concerns; in contrast, lru_cache has thread-safety provisions built in. There's benefit in keeping the Expression objects as simple immutable pure data structures - I'd rather not change that unless there's clear evidence that it's beneficial performance-wise.

No test changes or additions, because there are no observable behaviour changes.

Closes #1510

See also #1516, #1517

@mdickinson mdickinson merged commit d89adfb into main Sep 16, 2021
@mdickinson mdickinson deleted the performance/cache-compile-expr-result branch September 16, 2021 12:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cache observer expression's graphs
2 participants