Skip to content

Conversation

av-runner
Copy link

@av-runner av-runner commented Sep 2, 2025

Description

This improves the performance in big escape decoding and encoding test. Instead of creating the escape input 10 times in a loop, the string is created outside the loop once.

@av-runner av-runner changed the title Move variables outside of the for loop to improve performance PERF: Move variables outside of the for loop to improve performance Sep 2, 2025
escape_input = base * 1024 * 1024 * 2
for _ in range(10):
base = "\u00e5".encode()
escape_input = base * 1024 * 1024 * 2
Copy link
Member

Choose a reason for hiding this comment

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

do we know why theses are defined here in the first place? the looping nature of the test makes me think it could be intentional. e.g. having id(base) change every time might matter

Copy link
Author

Choose a reason for hiding this comment

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

I could not find anything as to why that might be needed.

Copy link
Contributor

Choose a reason for hiding this comment

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

The oldest commit that I could find regarding this test was 73763af, and the reassignment inside the loop does seem intentional to stress test the decoder.

Copy link
Contributor

github-actions bot commented Oct 3, 2025

This pull request is stale because it has been open for thirty days with no activity. Please update and respond to this comment if you're still interested in working on this.

@github-actions github-actions bot added the Stale label Oct 3, 2025
@mroeschke
Copy link
Member

Seems like this PR is stale and the test structure is intentional so closing

@mroeschke mroeschke closed this Oct 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants