Skip to content
This repository has been archived by the owner on Sep 14, 2023. It is now read-only.

Try to make perf improve #17

Merged
merged 2 commits into from
Jul 5, 2018
Merged

Conversation

wdv4758h
Copy link
Contributor

@wdv4758h wdv4758h commented Jul 4, 2018

No description provided.

@wdv4758h
Copy link
Contributor Author

wdv4758h commented Jul 4, 2018

um, forget to pull the new master

@wdv4758h wdv4758h force-pushed the try-to-make-perf-improve branch from 47cbc90 to c01aa15 Compare July 4, 2018 13:55
@wdv4758h
Copy link
Contributor Author

wdv4758h commented Jul 4, 2018

um, wha't wrong with the CI
error

@mre
Copy link
Owner

mre commented Jul 4, 2018

Sorry, guess it was my fault with 3a35e1a
Can you uncomment the cdylib line and commit that to this branch?
Lesson learned: will always go through a PR now. 😅

@wdv4758h
Copy link
Contributor Author

wdv4758h commented Jul 4, 2018

I just notice that when you reply 😄

@wdv4758h
Copy link
Contributor Author

wdv4758h commented Jul 4, 2018

this to_string remove looks like compile fine 😛

@mre
Copy link
Owner

mre commented Jul 4, 2018

In my local tests I can't see a big performance improvement yet. Actually the values are a bit worse compared to before. Now I know that this should not be the case and I guess the benchmark code is just very inaccurate. Therefore I created #18 to port the benchmarks to pytest-benchmark.

In any case, your change looks good and I think we should merge it, because we avoid a to_string() operation (and there can be lots of them). So +1 from my side!
Thanks!

@wdv4758h
Copy link
Contributor Author

wdv4758h commented Jul 4, 2018

I think pytest-benchmark will be a good one, really love the pytest ecosystem, awesome tools and easy to install.

@mre mre merged commit dcdf32e into mre:master Jul 5, 2018
@mre mre mentioned this pull request Jul 5, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants