Skip to content

Conversation

jakobnissen
Copy link
Member

@jakobnissen jakobnissen commented Sep 19, 2025

This was orignially a larger PR, but I've shelved the larger refactoring for now.
This PR does two things:

  • Add a new method tryparse(::Type{T}, c::AbstractChar; base::Integer=10) where {T <: Integer} which was previously missing
  • Speed up parsing of chars to integer by ~4x

Benchmark:
```
julia> s = [rand('0':'9') for i in 1:100000];

julia> function foo(s)
           n = 0
           for i in s
               n += parse(Int, i)
           end
           n
       end
```
Before: 196 us
After: 39 us
@tecosaur
Copy link
Member

Oi, you've sniped me! (just kidding, improvement is nice to see)

@nsajko nsajko added the performance Must go faster label Sep 19, 2025
@jakobnissen jakobnissen changed the title WIP: Refactor parsing integers/floats Improve char to int parsing, and add tryparse method Sep 21, 2025
@jakobnissen jakobnissen marked this pull request as ready for review September 21, 2025 13:11
@jakobnissen
Copy link
Member Author

Test failures are unrelated

Copy link
Member

@LilithHafner LilithHafner left a comment

Choose a reason for hiding this comment

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

This PR introduces a lot of unrelated and unmotivated style chagnes that make this function out of like with they style of the surrounding code. Other than that, this seems pretty reasonable.

@jakobnissen
Copy link
Member Author

  • I've aligned formatting closer with the original style (if only there was some automatic tool to format code so people didn't have to think about this!) 😉
  • I've changed the 0x0a literal to UInt8(10) as requested

Comment on lines +41 to +45
@noinline function _invalid_base(base)
throw(ArgumentError("invalid base: base must be 2 ≤ base ≤ 62, got $base"))
end

@noinline _invalid_digit(base, char) = throw(ArgumentError("invalid base $base digit $(repr(char))"))
Copy link
Member

Choose a reason for hiding this comment

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

These are weirdly inconsistent?

Copy link
Member Author

Choose a reason for hiding this comment

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

One is longer than the other, and therefore the line needs to be broken up.

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

Successfully merging this pull request may close these issues.

4 participants