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

Simplify Client leak reporting. #485

Merged
merged 1 commit into from
Mar 21, 2023

Conversation

zenhack
Copy link
Contributor

@zenhack zenhack commented Mar 21, 2023

This patch:

  • Simplifies the metadata we collect; we have a full stack trace now, so the rest is redundant.
  • Refactors so that dead code elimination can more easily drop the relevant code if SetClientLeakFunc is never called.
  • Remove the metadata from the clients themselves, which will reduce the memory usage of clients by a bit when reporting is off.

I'm not entirely sure what moved me to work on this now, but here we are.

@zenhack zenhack requested a review from lthibault March 21, 2023 04:50
This patch:

- Simplifies the metadata we collect; we have a full stack trace now, so
  the rest is redundant.
- Refactors so that dead code elimination can more easily drop the
  relevant code if SetClientLeakFunc is never called.
- Remove the metadata from the clients themselves, which will reduce
  the memory usage of clients by a bit when reporting is off.

I'm not entirely sure what moved me to work on this now, but here we
are.
Copy link
Collaborator

@lthibault lthibault left a comment

Choose a reason for hiding this comment

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

Sometimes "no reason" is the best reason 😉

LGTM!

@zenhack zenhack merged commit 2720214 into capnproto:main Mar 21, 2023
@zenhack zenhack deleted the simplify-leak-reporting branch March 21, 2023 15:56
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