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

faster replace on Dict and Set #33739

Merged
merged 1 commit into from
Oct 10, 2020
Merged

faster replace on Dict and Set #33739

merged 1 commit into from
Oct 10, 2020

Conversation

rfourquet
Copy link
Member

This optimizes replace and replace! functions on Dict (and Set) based on the internals. The most spectacular improvements is when dict elements are replaced with another element with the same key: then the old value can be overwritten directly, and the time taken by replace basically doesn't depend anymore on the number of replaced elements, just the length of the dict.
The following table shows examples of speed-up for different proportions of replaced elements:

replace replace!
10% elements replaced 2x 2.5x
50% 3x 7x
90% 5x 14x

When the keys change, the speedup seems to be generally between 30 and 40%. Not as nice, but still a modest improvement.

@rfourquet rfourquet added collections Data structures holding multiple items, e.g. sets performance Must go faster labels Nov 1, 2019
@StefanKarpinski
Copy link
Member

Bump. Who might be good to review this?

@rfourquet
Copy link
Member Author

Bump

@rfourquet
Copy link
Member Author

I just reviewed in detail, LGTM ;-)
I'm tempted to merge soon...

@KristofferC
Copy link
Member

I'm tempted to merge soon...

Just make sure CI pass first :P

@rfourquet
Copy link
Member Author

Just make sure CI pass first :P

Oh right! since this PR, I fixed a bug 6 months ago (#35450) with new tests which fail here, because of a missing count == 0 && return res. Added this condition now, let's see what CI says now...

@rfourquet
Copy link
Member Author

rfourquet commented Oct 5, 2020

There is one unrelated test failure with Dowloads (https://build.julialang.org/#/builders/33/builds/4397/steps/5/logs/stdio) :

Downloads: Test Failed at /usr/home/julia/buildbot/worker-tabularasa/tester_freebsd64/build/share/julia/stdlib/v1.6/Downloads/test/runtests.jl:82
  Expression: 2t < count * delay
   Evaluated: 21.39263454 < 20

cc @StefanKarpinski

@rfourquet rfourquet merged commit 8a52ea4 into master Oct 10, 2020
@rfourquet rfourquet deleted the rf/replace-Dict branch October 10, 2020 12:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
collections Data structures holding multiple items, e.g. sets performance Must go faster
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants