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

Optimize ISO datetime parsing using binary pattern matching with nibbles #14177

Closed

Conversation

dkuku
Copy link
Contributor

@dkuku dkuku commented Jan 11, 2025

I promise this is the last one. To be honest, I started with this idea, but I found other parts of this module that benefited more from the changes. However, I've had this in the back of my mind and needed to at least propose it for discussion.

This PR optimizes the parsing of ISO format dates and times by leveraging more efficient binary pattern matching. Key changes include:

  • The pattern 3::4 matches the high nibble of ASCII digits (0x3_), while the value::4 captures the actual digit value directly, eliminating the need for ASCII offset arithmetic
  • Simplified numeric conversion by removing the need for ASCII offset subtraction (?0)
  • Simplified guard clauses by removing redundant >= 0 checks since nibble matching guarantees values are non-negative

The measured performance is almost the same, but there is a noticeable reduction in memory usage (approximately 40 bytes - correlating with the removal of zero guards and subtraction operations).

Note: This optimization was previously considered for nimble_parsec but showed suboptimal performance on BEAM versions prior to OTP 26.

Name                                          ips        average  deviation         median         99th %                                                                                                                                 
Calendar.IOISO.parse_naive_datetime        4.64 M      215.58 ns  ±8027.76%         170 ns         310 ns                                                                                                                                 
Calendar.ISO.parse_naive_datetime          4.58 M      218.56 ns  ±7400.55%         180 ns         320 ns                                                                                                                                 
                                                                                                                                                                                                                                          
Comparison:                                                                                                                                                                                                                               
Calendar.IOISO.parse_naive_datetime        4.64 M                                                                                                                                                                                         
Calendar.ISO.parse_naive_datetime          4.58 M - 1.01x slower +2.97 ns                                                                                                                                                                 
                                                                                                                                                                                                                                          
Extended statistics:                                                                                                                                                                                                                      
                                                                                                                                                                                                                                          
Name                                        minimum        maximum    sample size                     mode                                                                                                                                
Calendar.IOISO.parse_naive_datetime          150 ns    23428853 ns         5.42 M                   170 ns                                                                                                                                
Calendar.ISO.parse_naive_datetime            150 ns    20037235 ns         5.33 M                   180 ns                                                                                                                                
                                                                                                                                                                                                                                          
Memory usage statistics:                                                                                                                                                                                                                  
                                                                                                                                                                                                                                          
Name                                   Memory usage                                                                                                                                                                                       
Calendar.IOISO.parse_naive_datetime           680 B                                                                                                                                                                                       
Calendar.ISO.parse_naive_datetime             720 B - 1.06x memory usage +40 B                                                                                                                                                            
                                                                                                                                                                                                                                          
**All measurements for memory usage were the same**                                                                                                                                                                                       
                                                                                                                                                                                                                                          
Reduction count statistics:                                                                                                                                                                                                               
                                                                                                                                                                                                                                          
Name                                Reduction count                                                                                                                                                                                       
Calendar.IOISO.parse_naive_datetime              45                                                                                                                                                                                       
Calendar.ISO.parse_naive_datetime                45 - 1.00x reduction count +0                                                                                                                                                            

@josevalim
Copy link
Member

Thank you! I would like to hear opinions on this one, because it may be that we find the code more confusing and the gain is small.

@dkuku
Copy link
Contributor Author

dkuku commented Jan 11, 2025

I know, this is a good candidate for a macro or to include it in the core:
<<h1::ascii_int, h2::ascii_int, _::binary>>
For example ::utf8/16/32 can also not not match on every binary
and little/big modify the returned data.
You could even match on multiple digits and optimize the result:
<<year::ascii_int(4), _rest::binary>>

@josevalim
Copy link
Member

josevalim commented Jan 22, 2025

@dkuku why don't we give such a macro a try then on this change?

defmacrop ascii(x) do
  quote do
    <<3::4, unquote(x)::4>>
  end
end

And then it may clean up the code a bit?

@dkuku
Copy link
Contributor Author

dkuku commented Jan 22, 2025

@josevalim It works but I need to define it in a separate module because - probably because this code is not inside a function.
Also ascii does not fit here, maybe int fits better (you can compare both commits.)

@dkuku
Copy link
Contributor Author

dkuku commented Jan 22, 2025

The longer I think about it the more I think such match should be implemented in otp. Matching and parsing integers from binaries is much more common than utf.

@josevalim
Copy link
Member

Thank you for exploring this. I think between the verbosity and introducing a module only for this, we are stuck between a rock and a hard place, so it is best if we don't go ahead for the gains. Maybe we can have breakthroughs in the future and revisit this. Thank you!

@josevalim josevalim closed this Jan 22, 2025
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.

2 participants