Skip to content

Conversation

@alex
Copy link
Contributor

@alex alex commented Jun 1, 2016

classes are relatively expensive to create, and particularly disadvantageous on PyPy

A benchmark could be created if this was useful

@brian-brazil
Copy link
Contributor

There's two other Timer classes.

How big is the performance difference?

@alex
Copy link
Contributor Author

alex commented Jun 1, 2016

Using this simple benchmark:

from prometheus_client import Histogram


def main():
    h = Histogram("nonsense_latency_seconds", "Latency of nonsense (seconds)")

    for i in xrange(100000):
        with h.time():
            pass


if __name__ == "__main__":
    main()

On PyPy 5.1.1

WIth patch:

(tempenv-43511495362f0) ~ $ time python t.py
        0.22 real         0.17 user         0.04 sys
(tempenv-43511495362f0) ~ $ time python t.py
        0.23 real         0.18 user         0.04 sys
(tempenv-43511495362f0) ~ $ time python t.py
        0.23 real         0.18 user         0.05 sys

master:

(tempenv-43511495362f0) ~ $ time python t.py
        4.45 real         4.29 user         0.10 sys
(tempenv-43511495362f0) ~ $ time python t.py
        4.50 real         4.30 user         0.11 sys
(tempenv-43511495362f0) ~ $ time python t.py
        4.50 real         4.31 user         0.12 sys

So pretty massive. This optimization should be made to all the other nested Timer classes.

@brian-brazil
Copy link
Contributor

That's a pretty nice improvement, most of the slowness is due to the mutex at that point.

@alex
Copy link
Contributor Author

alex commented Jun 1, 2016

Indeed. Do you want me to move the other classes in this PR, or a separate one?

@brian-brazil
Copy link
Contributor

All in one PR please.

@alex
Copy link
Contributor Author

alex commented Jun 1, 2016

@brian-brazil done.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can remove this blank line now.

@brian-brazil
Copy link
Contributor

That looks generally fine, can you squash your commits please?

Classes are relatively expensive to create, and particularly disadvantageous on PyPy.
@alex
Copy link
Contributor Author

alex commented Jun 1, 2016

@brian-brazil done!

@brian-brazil brian-brazil merged commit f005a78 into prometheus:master Jun 1, 2016
@brian-brazil
Copy link
Contributor

Thanks!

@alex alex deleted the patch-1 branch June 1, 2016 15:51
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