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

Avoid Memory collision between notebooks with same function name #1097

Merged
merged 2 commits into from
Oct 2, 2020

Conversation

NicolasHug
Copy link
Contributor

Fixes #1096

This PR changes the cache directory from

[...]/joblib-__ipython-input__/<TheFunctionName>/

to

[...]/joblib-<ipython-input-XYZ>/<TheFunctionName>/,
where XYZ comes from co_filename and is a hash of the function.

Ideally this should probably use the .ipynb filename but I don't think we can retrieve it easily, and it wouldn't work for pure IPython sessions.

I'm not sure how to test this?

@codecov
Copy link

codecov bot commented Aug 24, 2020

Codecov Report

Merging #1097 into master will decrease coverage by 0.61%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1097      +/-   ##
==========================================
- Coverage   94.52%   93.90%   -0.62%     
==========================================
  Files          47       47              
  Lines        6955     6956       +1     
==========================================
- Hits         6574     6532      -42     
- Misses        381      424      +43     
Impacted Files Coverage Δ
joblib/func_inspect.py 91.07% <100.00%> (-1.15%) ⬇️
joblib/test/test_memory.py 98.41% <100.00%> (-0.15%) ⬇️
joblib/backports.py 44.73% <0.00%> (-39.48%) ⬇️
joblib/_memmapping_reducer.py 94.33% <0.00%> (-2.27%) ⬇️
joblib/test/test_memmapping.py 97.33% <0.00%> (-1.91%) ⬇️
joblib/hashing.py 89.38% <0.00%> (-1.77%) ⬇️
joblib/disk.py 90.47% <0.00%> (-1.59%) ⬇️
joblib/logger.py 85.52% <0.00%> (-1.32%) ⬇️
joblib/pool.py 86.99% <0.00%> (-0.82%) ⬇️
... and 3 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 440b1b8...28604e7. Read the comment docs.

@@ -128,10 +128,20 @@ def get_func_name(func, resolv_alias=True, win_characters=True):
# mangling of full path to filename
parts = filename.split(os.sep)
if parts[-1].startswith('<ipython-input'):
# function is defined in an IPython session. The filename
# will change with every new kernel instance. This hack
Copy link
Contributor Author

Choose a reason for hiding this comment

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

BTW, I believe the comment is incorrect: the filename doesn't change with new kernel instances. It changes because of N (see new comments)

@ogrisel
Copy link
Contributor

ogrisel commented Oct 2, 2020

Thanks for the fix. Merging.

@ogrisel ogrisel merged commit 49a2c10 into joblib:master Oct 2, 2020
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.

Memory: cache hit collision with same function name but different notebooks
2 participants