Skip to content

fix memory leak on job execution from baseservice#240

Merged
bgentry merged 1 commit intomasterfrom
bg-fix-baseservice-rand-mem-leak
Feb 29, 2024
Merged

fix memory leak on job execution from baseservice#240
bgentry merged 1 commit intomasterfrom
bg-fix-baseservice-rand-mem-leak

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
@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