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

Use LRUCache in ExpressionParser for efficient memory usage #6667

Merged
merged 1 commit into from
Jul 11, 2023
Merged

Use LRUCache in ExpressionParser for efficient memory usage #6667

merged 1 commit into from
Jul 11, 2023

Conversation

ramfattah
Copy link
Contributor

Fixes #6656

Description

This PR addresses a memory leak issue in the ExpressionParser class. Previously, the ExpressionParser class used a ConcurrentDictionary to cache parsed expressions, which could lead to excessive memory usage over time. This PR changes the ExpressionParser class to use an LRUCache instead, which has a maximum capacity and uses a least-recently-used policy to evict entries from the cache when the maximum capacity is reached. This change should help to mitigate the memory leak issue.

Specific Changes

  • Changed the ExpressionParser class to use an LRUCache instead of a ConcurrentDictionary for caching parsed expressions.

Testing

This change was tested using a console application that demonstrates the memory leak issue in ExpressionParser:
https://github.com/jamesemann/expression-parser-leak

Before:

After:

Copy link
Collaborator

@ceciliaavila ceciliaavila left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@tracyboehrer tracyboehrer added the Automation: No parity PR does not need to be applied to other languages. label Jul 11, 2023
@tracyboehrer tracyboehrer merged commit b83ec68 into microsoft:main Jul 11, 2023
tracyboehrer added a commit that referenced this pull request Jul 13, 2023
* Upgrade Antlr4.Runtime to v.4.11.1 (#6670)

* Add WithAuthority to MSAL application level (#6671)

* Use LRUCache in ExpressionParser for efficient memory usage (#6667)

* Microsoft.Rest.ClientRuntime bump (#6662)

Co-authored-by: Tracy Boehrer <trboehre@microsoft.com>

* Updated NuGet.Packaging (#6652)

Co-authored-by: PVAShiproom <pvashiproom@microsoft.com>

---------

Co-authored-by: Cecilia Avila <44245136+ceciliaavila@users.noreply.github.com>
Co-authored-by: Joel Mut <62260472+sw-joelmut@users.noreply.github.com>
Co-authored-by: Ram Fattah <38049078+ramfattah@users.noreply.github.com>
Co-authored-by: Tracy Boehrer <trboehre@microsoft.com>
Co-authored-by: PVAShiproom <pvashiproom@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Automation: No parity PR does not need to be applied to other languages.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

GC root in ExpressionParser (AdaptiveExpressions)
3 participants