Skip to content

script: Free memory earlier in hot path of input handling#43226

Merged
TimvdLippe merged 1 commit intoservo:mainfrom
yezhizhen:hot-path
Mar 13, 2026
Merged

script: Free memory earlier in hot path of input handling#43226
TimvdLippe merged 1 commit intoservo:mainfrom
yezhizhen:hot-path

Conversation

@yezhizhen
Copy link
Copy Markdown
Member

@yezhizhen yezhizhen commented Mar 13, 2026

This is a hot path. This way we free the unused memory earlier.
Didn't realize the difference in #43202.

Testing: This is a memory optimization that does not change visible behaviour.

Signed-off-by: Euclid Ye <[email protected]>
@yezhizhen yezhizhen requested a review from gterzian as a code owner March 13, 2026 01:02
@servo-highfive servo-highfive added the S-awaiting-review There is new code that needs to be reviewed. label Mar 13, 2026
@yezhizhen
Copy link
Copy Markdown
Member Author

It seems this is guaranteed to be better for all such occurrences:

  1. The variable is temporary
  2. We are not appending to the container again.

Should we turn this into a "everywhere: ..." change?

@xiaochengh
Copy link
Copy Markdown
Contributor

It seems this is guaranteed to be better for all such occurrences:

  1. The variable is temporary
  2. We are not appending to the container again.

Should we turn this into a "everywhere: ..." change?

Nice observation! And yes we should do that.

Let's start with filing an issue to replace drain with mem::take with an explanation why the latter is better.

@servo-highfive servo-highfive removed the S-awaiting-review There is new code that needs to be reviewed. label Mar 13, 2026
@TimvdLippe TimvdLippe added this pull request to the merge queue Mar 13, 2026
@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 Mar 13, 2026
Merged via the queue into servo:main with commit 97225f5 Mar 13, 2026
34 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 Mar 13, 2026
@yezhizhen yezhizhen deleted the hot-path branch March 17, 2026 10:13
github-merge-queue bot pushed a commit that referenced this pull request Apr 1, 2026
…he temporary container if it is unused (#43826)

Follow up of #43226
This also allows us to remove some `mut` declaration.
Fix some unintended Chinese quotation mark.

Testing: This is a micro-optimization which does not change visible
behaviour.

Signed-off-by: Euclid Ye <[email protected]>
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.

4 participants