Skip to content

script: cleanup memory reporting code inside dedicatedworkerglobalscope #40170

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Oct 26, 2025
Merged

script: cleanup memory reporting code inside dedicatedworkerglobalscope #40170

merged 3 commits into from
Oct 26, 2025
+11 −17

Conversation

@Gae24
Copy link
Contributor

@Gae24 Gae24 commented Oct 25, 2025

  • Remove a couple of ignore_malloc_size_of since are not required anymore
  • Avoid calling random and use the worker_id
  • Pass DedicatedWorker event_loop_sender to run_with_memory_profiling

Testing: Covered by tests, only code related to memory reporting was changed.
Fixes: #11855

Gae24 added 3 commits October 24, 2025 14:51
@Gae24
remove unnecessary ignore_malloc_size_of
228ce64 
Signed-off-by: Gae24 <96017547+Gae24@users.noreply.github.com>
@Gae24
use worker_id for memory reporting, remove outdated comment
f5a6263 
Signed-off-by: Gae24 <96017547+Gae24@users.noreply.github.com>
@Gae24
dedicatedworkerglobalscope: use event loop sender for memory reporting
25199d3 
Signed-off-by: Gae24 <96017547+Gae24@users.noreply.github.com>
@Gae24 Gae24 requested a review from gterzian as a code owner October 25, 2025 17:39
@servo-highfive servo-highfive added the S-awaiting-review There is new code that needs to be reviewed. label Oct 25, 2025
@Gae24
Copy link
Contributor Author

Gae24 commented Oct 25, 2025

Tested with workers/semantics/multiple-workers/002.html
Before
memory-test-old
Now
memory-test-new

I'm not sure why system-heap-unclassified report more memory now

@jdm
Copy link
Member

jdm commented Oct 26, 2025

The unclassified number is the total allocation size minus the explicit reports. If we're not longer double counting, I am not surprised to see that number go up.

jdm
jdm approved these changes Oct 26, 2025
View reviewed changes
Copy link
Member

@jdm jdm left a comment

Choose a reason for hiding this comment

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

Thank you!

@servo-highfive servo-highfive removed the S-awaiting-review There is new code that needs to be reviewed. label Oct 26, 2025
@jdm jdm added this pull request to the merge queue Oct 26, 2025
@servo-highfive servo-highfive added the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Oct 26, 2025
Merged via the queue into servo:main with commit af6b62d Oct 26, 2025
33 checks passed
@servo-highfive servo-highfive removed the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Oct 26, 2025
@jdm jdm mentioned this pull request Oct 26, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jdm jdm jdm approved these changes

@gterzian gterzian Awaiting requested review from gterzian gterzian is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

DedicatedWorkerGlobalScope::run_worker_scope shouldn't use parent_sender for run_with_memory_profiling

3 participants

@Gae24 @jdm @servo-highfive