Skip to content

Conversation

@groutr
Copy link
Contributor

@groutr groutr commented Dec 6, 2018

The existing, recursive, form of update_in is unnecessarily slow. This borrows the non-recursive implementation from cytoolz and shows a good performance boost.

from toolz import update_in, update_in2
keys1 = list(reversed([list(range(i+100)) for i in range(100)]))
keys2 = list(reversed([list(range(i)) for i in range(1, 11)]))

# A custom assoc_in function which simply allows me to 
# change which update_in function is called.

%%timeit
d = {}
for x in keys1:
    d = assoc_in(update_in, d, x, 42)
# update_in (keys1)
26.3 ms ± 668 µs per loop (mean ± std. dev. of 7 runs, 10 loops each)
# update_in2 (keys1)
4.96 ms ± 62.2 µs per loop (mean ± std. dev. of 7 runs, 100 loops each)

# update_in (keys2)
86.7 µs ± 2.48 µs per loop (mean ± std. dev. of 7 runs, 10000 loops each)
# update_in2 (keys2)
21.4 µs ± 118 ns per loop (mean ± std. dev. of 7 runs, 10000 loops each)

@groutr
Copy link
Contributor Author

groutr commented Dec 6, 2018

ping @eriknw

@groutr
Copy link
Contributor Author

groutr commented Dec 18, 2018

@eriknw do you have any feedback on this change?

@eriknw
Copy link
Member

eriknw commented Jun 22, 2019

LGTM, thanks @groutr! Are there any other speed improvements to steal from cytoolz?

Also, aesthetically, this uses more blank lines then I generally prefer, but I'd like to get this in, so merging!

@eriknw eriknw merged commit cde596b into pytoolz:master Jun 22, 2019
@groutr
Copy link
Contributor Author

groutr commented Jun 24, 2019

@eriknw I'm sure there are probably other speed improvements that could be carried over from cytoolz, but I haven't looked in awhile. Thanks for merging!

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.

2 participants