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

Memorise branch #62

Merged
merged 10 commits into from
Nov 28, 2014
Merged

Memorise branch #62

merged 10 commits into from
Nov 28, 2014

Conversation

matsjoyce
Copy link
Contributor

Fixes for #49 and #43.

Latest performance:

Operation Memorise time Master time Slow down
Import dill 0.16855 0.09794 1.721
Dump small python module 0.00242 0.00029 8.213
Dump small python module which imports sys 0.00281 0.00033 8.404
Dump large python module 0.02601 0.00883 2.945
Dump large python module with attr changed 0.02548 0.00871 2.925

I've replaced abc with a large, non builtin module, as all master does with builtin modules is pickle a ref, which make comparison difficult.

@matsjoyce
Copy link
Contributor Author

This should probably only be activated explicitly (dill.use_diff(on=True)), by people which want this behaviour, as this will help with compatabillity, etc.

@mmckerns
Copy link
Member

mmckerns commented Oct 4, 2014

that's a decent idea to get it into the trunk, at least.

@matsjoyce
Copy link
Contributor Author

OK, done that. Also renamed to diff.py, because that will make more sense. The current implementation will only add overhead if the use_diff function is called. The only "bad" point is that it introduces another global variable, _use_diff.

@mmckerns mmckerns modified the milestone: dill-0.2.2 Nov 25, 2014
mmckerns added a commit that referenced this pull request Nov 28, 2014
@mmckerns mmckerns merged commit 51285bb into uqfoundation:master Nov 28, 2014
@mmckerns
Copy link
Member

merged, but disabled in d8d566c, as the diff module is a bit raw. Will post issues as new tickets.

@matsjoyce matsjoyce deleted the memorise branch November 29, 2014 17:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants