Skip to content

Conversation

@groutr
Copy link
Contributor

@groutr groutr commented Aug 10, 2018

Using itertools.islice seems to be slower than simply calling next twice. In each case, calling next twice is faster.

m0 = range(5)
m1 = range(100)
m2 = range(10000)

In [17]: %timeit second(m0)
298 ns ± 5.18 ns per loop (mean ± std. dev. of 7 runs, 1000000 loops each)

In [18]: %timeit second(m1)
296 ns ± 4.49 ns per loop (mean ± std. dev. of 7 runs, 1000000 loops each)

In [19]: %timeit second(m2)
290 ns ± 3.98 ns per loop (mean ± std. dev. of 7 runs, 1000000 loops each)
===============================================
In [21]: %timeit my_second(m0)
263 ns ± 6.78 ns per loop (mean ± std. dev. of 7 runs, 1000000 loops each)

In [22]: %timeit my_second(m1)
257 ns ± 2.12 ns per loop (mean ± std. dev. of 7 runs, 1000000 loops each)

In [23]: %timeit my_second(m3)
263 ns ± 6.2 ns per loop (mean ± std. dev. of 7 runs, 1000000 loops each)

@eriknw
Copy link
Member

eriknw commented Aug 14, 2018

Nice. This is what cytoolz does as well. Should we add a try/except block as was needed for #409?

@groutr
Copy link
Contributor Author

groutr commented Aug 14, 2018

@eriknw, you're right, we need a try/except around the next() call. And interpose needs to be changed as well. I'll do that in another PR.

The try/except might negate the performance gain here though.

Add more tests for edge cases
@groutr
Copy link
Contributor Author

groutr commented Aug 15, 2018

In [21]: %timeit second(m0) # Released implementation
385 ns ± 9.37 ns per loop (mean ± std. dev. of 7 runs, 1000000 loops each)
In [22]: %timeit second(m1)
382 ns ± 12 ns per loop (mean ± std. dev. of 7 runs, 1000000 loops each)
In [23]: %timeit second(m2)
378 ns ± 2.93 ns per loop (mean ± std. dev. of 7 runs, 1000000 loops each)
In [25]: %timeit my_second(m0)  # Final version with try/except
365 ns ± 7.99 ns per loop (mean ± std. dev. of 7 runs, 1000000 loops each)
In [26]: %timeit my_second(m1)
359 ns ± 4.69 ns per loop (mean ± std. dev. of 7 runs, 1000000 loops each)
In [27]: %timeit my_second(m2)
362 ns ± 5.31 ns per loop (mean ± std. dev. of 7 runs, 1000000 loops each)

@groutr
Copy link
Contributor Author

groutr commented Aug 15, 2018

@eriknw, testing on Python 3.7, I don't think second needs the fix from #409 because second is not a generator. I'm reverting the last commit.

@eriknw
Copy link
Member

eriknw commented Aug 15, 2018

heh, my bad. LGTM. Thanks @groutr!

@eriknw eriknw merged commit 68d8df8 into pytoolz:master Aug 15, 2018
@groutr
Copy link
Contributor Author

groutr commented Aug 15, 2018

@eriknw no worries. It made me read the PEP a little more thoroughly.

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