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

fix memory leak on job execution from baseservice #240

Merged
merged 1 commit into from
Feb 29, 2024

Conversation

bgentry
Copy link
Contributor

@bgentry bgentry commented Feb 29, 2024

The BaseService type was originally only used for long-running internal services. At some point, we refactored the job execution code to also make use of this type. That exposed a memory leak: every time a new BaseService is Init'd, a new random source is allocated. That meant for every single job.

This is of course totally unnecessary, and to avoid it we can move the rand source to the Archetype, which is only allocated once per client.

Partially fixes #239.

@bgentry bgentry requested a review from brandur February 29, 2024 02:42
@bgentry bgentry force-pushed the bg-fix-baseservice-rand-mem-leak branch from 9805316 to 80432fb Compare February 29, 2024 02:44
@bgentry bgentry mentioned this pull request Feb 29, 2024
Copy link
Contributor

@brandur brandur left a comment

Choose a reason for hiding this comment

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

Awesome. Thanks for the fix!

@bgentry bgentry force-pushed the bg-fix-baseservice-rand-mem-leak branch from 80432fb to 95ecd58 Compare February 29, 2024 02:59
@bgentry bgentry enabled auto-merge (squash) February 29, 2024 03:01
The `BaseService` type was originally only used for long-running
internal services. At some point, we refactored the job execution code
to also make use of this type. That exposed a memory leak: every time a
new `BaseService` is `Init`'d, a new random source is allocated. That
meant for every single job.

This is of course totally unnecessary, and to avoid it we can move the
rand source to the `Archetype`, which is only allocated once per client.

Partially fixes #239.
@bgentry bgentry force-pushed the bg-fix-baseservice-rand-mem-leak branch from 95ecd58 to fb8ff63 Compare February 29, 2024 03:05
@bgentry bgentry merged commit 8534662 into master Feb 29, 2024
10 checks passed
@bgentry bgentry deleted the bg-fix-baseservice-rand-mem-leak branch February 29, 2024 03:12
brandur added a commit that referenced this pull request Feb 29, 2024
Tee up the `v0.0.23` release. This contains a large refactor in #212
that changes substantial code, and an important memory leak fix from #240.
brandur added a commit that referenced this pull request Feb 29, 2024
Tee up the `v0.0.23` release. This contains a large refactor in #212
that changes substantial code, and an important memory leak fix from #240.
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.

Memory leak?
2 participants