Skip to content

Conversation

@firewave
Copy link
Collaborator

No description provided.

@firewave

This comment was marked as outdated.

@firewave

This comment was marked as outdated.

@firewave

This comment was marked as outdated.

@firewave firewave force-pushed the emptystring-z branch 4 times, most recently from 5c240c0 to 5ebf215 Compare March 11, 2025 08:26
@firewave firewave force-pushed the emptystring-z branch 4 times, most recently from 6d5b102 to 9dc139b Compare March 13, 2025 18:47
@firewave firewave force-pushed the emptystring-z branch 3 times, most recently from bf04e79 to cd074a9 Compare August 27, 2025 13:04
@firewave
Copy link
Collaborator Author

#7772 addresses part of the regression. The rest seems mostly stemming from the default parameters for Token::insertToken().

@firewave
Copy link
Collaborator Author

#7813 added more mitigations for the regressions from this change. But I still see a minor regression I need to look into.

@firewave firewave force-pushed the emptystring-z branch 3 times, most recently from 812cbf1 to ad89c58 Compare October 22, 2025 19:50
@firewave
Copy link
Collaborator Author

The remaining regression just seems to be related to slight differences in the determined Ir count for very hot calls. I see no increase in calls.

@firewave firewave marked this pull request as ready for review October 22, 2025 19:56
@sonarqubecloud
Copy link

Copy link
Owner

@danmar danmar left a comment

Choose a reason for hiding this comment

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

I think we had emptyString for performance reasons.. so how much has it slowed down?

@firewave
Copy link
Collaborator Author

I think we had emptyString for performance reasons.. so how much has it slowed down?

If you read my comments above you will see that I addressed the regressions it caused and that it hasn't slowed down.

@firewave firewave merged commit 36dbff5 into danmar:main Oct 28, 2025
54 checks passed
@firewave firewave deleted the emptystring-z branch October 28, 2025 19:29
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.

2 participants