Skip to content

Use WeakKeyDict for the subexpression dictionary - #4207

Open
odow wants to merge 4 commits into
masterfrom
od/hash2
Open

Use WeakKeyDict for the subexpression dictionary#4207
odow wants to merge 4 commits into
masterfrom
od/hash2

Conversation

@odow

@odow odow commented Jul 24, 2026

Copy link
Copy Markdown
Member

A variant of #4204 that closes #4203

The downside to this approach is that we need to decide how frequently to filter the subexpression dictionary. If an element has been GC'd, it cannot be referenced in the future, so it is safe to remove.

@codecov

codecov Bot commented Jul 24, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.96%. Comparing base (844a2f5) to head (af5d201).
⚠️ Report is 2 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4207      +/-   ##
==========================================
+ Coverage   99.95%   99.96%   +0.01%     
==========================================
  Files          43       43              
  Lines        6306     6313       +7     
==========================================
+ Hits         6303     6311       +8     
+ Misses          3        2       -1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Comment thread src/JuMP.jl Outdated
Comment thread src/JuMP.jl Outdated

Base.get(cache::_WeakCache, key, default) = get(cache.data, key, default)

Base.getindex(cache::_WeakCache, key) = getindex(cache.data, WeakRef(key))

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Where is this used ?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

It was used in the tests to simplify things. I guess I can remove.

@odow

odow commented Jul 28, 2026

Copy link
Copy Markdown
Member Author

I think I'm just re-implementing Base.WeakKeyDict. Except that instead of having a finaliser on the key, I have the clean-up pass.

@odow odow changed the title Use WeakRef as the keys for the subexpression dictionary Use WeakKeyDict for the subexpression dictionary Jul 28, 2026

julia> @allocated @expression(model, @force_nonlinear(x * 2.0 * (1 + x) * x))
944
784

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Surprising that this decreases when we change struct to mutable struct

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

Potential hash collision for nonlinear subexpressions

2 participants