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

Add GC-related metrics #968

Merged
merged 2 commits into from
Nov 8, 2018
Merged

Conversation

skhoroshavin
Copy link
Member

Signed-off-by: Sergey Khoroshavin sergey.khoroshavin@dsr-corporation.com

Signed-off-by: Sergey Khoroshavin <sergey.khoroshavin@dsr-corporation.com>
@@ -121,6 +120,25 @@
logger = getlogger()


class GcTimeTracker:
Copy link
Contributor

Choose a reason for hiding this comment

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

How this can affect performance? Should we make this metrics disabled by default?

Copy link
Member Author

Choose a reason for hiding this comment

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

Adding callback doesn't measurably affect performance. Asking for gc.get_objects() does add performance spikes once per 10 seconds, but only when there are already lots of objects, in which case we already have much bigger spikes from GC itself, so I don't think this is critical.

Copy link
Member Author

Choose a reason for hiding this comment

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

On a second thought it's not hard to add checks if metrics are enabled, so I'll add them anyways.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

Signed-off-by: Sergey Khoroshavin <sergey.khoroshavin@dsr-corporation.com>
@ashcherbakov ashcherbakov merged commit 7ddbeb3 into hyperledger:master Nov 8, 2018
@skhoroshavin skhoroshavin deleted the gc-metrics branch November 8, 2018 12:59
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.

None yet

2 participants