Skip to content

Conversation

@overlookmotel
Copy link
Member

@overlookmotel overlookmotel commented Oct 13, 2024

Small optimization. Use AsciiChar (introduced in #6537) for pushing bytes to buffer where the byte values are not statically known.

This also allows removing some unsafe code.

@graphite-app
Copy link
Contributor

graphite-app bot commented Oct 13, 2024

Your org has enabled the Graphite merge queue for merging into main

Add the label “0-merge” to the PR and Graphite will automatically add it to the merge queue when it’s ready to merge. Or use the label “hotfix” to add to the merge queue as a hot fix.

You must have a Graphite account and log in to Graphite in order to use the merge queue. Sign up using this link.

Copy link
Member Author

overlookmotel commented Oct 13, 2024

@codspeed-hq
Copy link

codspeed-hq bot commented Oct 13, 2024

CodSpeed Performance Report

Merging #6538 will not alter performance

Comparing 10-13-perf_codegen_use_asciichar_for_statically_unknowable_characters (21226b2) with 10-13-feat_data_structures_introduce_asciichar_ (50e6c68)

Summary

✅ 29 untouched benchmarks

@overlookmotel
Copy link
Member Author

Weird performance effects. I'll look into it.

@overlookmotel overlookmotel force-pushed the 10-13-feat_data_structures_introduce_asciichar_ branch from 8c8c4d2 to 76691be Compare October 13, 2024 22:02
@overlookmotel overlookmotel force-pushed the 10-13-feat_data_structures_introduce_asciichar_ branch from 76691be to dfcc8eb Compare October 13, 2024 22:09
@overlookmotel overlookmotel force-pushed the 10-13-perf_codegen_use_asciichar_for_statically_unknowable_characters branch from 2ba1198 to 75fedc6 Compare October 13, 2024 22:09
@Boshen
Copy link
Member

Boshen commented Oct 14, 2024

This is making the code way too complicated, can I reject?

@Boshen Boshen force-pushed the 10-13-feat_data_structures_introduce_asciichar_ branch from dfcc8eb to 50e6c68 Compare October 14, 2024 01:34
@Boshen Boshen force-pushed the 10-13-perf_codegen_use_asciichar_for_statically_unknowable_characters branch from 75fedc6 to 21226b2 Compare October 14, 2024 01:35
@overlookmotel
Copy link
Member Author

This is making the code way too complicated, can I reject?

Yes, I see your point. Too much complexity for not enough gain.

@Boshen Boshen deleted the 10-13-perf_codegen_use_asciichar_for_statically_unknowable_characters branch November 20, 2024 13:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-codegen Area - Code Generation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants