Skip to content
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.

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

Re-port new integer-scanning utility methods #127

Merged
merged 5 commits into from
Dec 13, 2024

Conversation

headius
Copy link
Contributor

@headius headius commented Dec 12, 2024

This is a re-port of the C code from @byroot for fast base 10 and base 16 integer scanning.

In #125, @kou pointed out there's an intermittent failure in the JRuby extension. We were unable to confirm exactly the circumstances that cause that failure, but this re-port should at least help reduce the change it is a bug in the original Java code.

This may help prevent ArrayIndexOutOfBounds randomly seen in CI.

See ruby#125
* Parse base should be from current pointer so add that to the get
  calls.
* Pull out ascii check into util method.
* Rename curr local var to ptr to better match C version.
This is to align the base10 code with the freshly-ported base16
code.

See ruby#125
@headius
Copy link
Contributor Author

headius commented Dec 12, 2024

@kou @byroot this should be ready to go but I have one concern about the C code...

It doesn't seem to update curr after the parse? I have that in the Java code because it seemed necessary.

This is temporary until we're sure that the AIOOB from
ruby#125 has been fixed by ruby#127.
@byroot
Copy link
Member

byroot commented Dec 12, 2024

It doesn't seem to update curr after the parse?

It does inside strscan_parse_integer

@headius
Copy link
Contributor Author

headius commented Dec 12, 2024

It does inside strscan_parse_integer

Aha, so it does! I will tweak this to move the final integer parse and curr update into a similar method, so it aligns better with the C code.

This aligns with C code that does the final parse and curr update
in strscan_parse_integer.
@kou kou merged commit ea0786b into ruby:master Dec 13, 2024
37 checks passed
@kou
Copy link
Member

kou commented Dec 13, 2024

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants