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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

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>>

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