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

DRAFT: Fast paths for large, native strings #646

Draft
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

milseman
Copy link
Member

@milseman milseman commented Apr 3, 2023

Pulls in a bunch of String-internal bit fiddly code in order to quickly get a pointer to contiguous UTF-8 if we're a (large) native string.

Extract out the Metrics data (unused in release builds), as it seems like having Processor surpass the 256-byte size causes significant regressions for some reason. (see milseman#1).

The below results are post- #642 and post- #644

=== Regressions ======================================================================
- EmojiRegexAll                           72.9ms	71.9ms	1.03ms		1.4%
- SubtractionCCC                          21.7ms	21.3ms	314µs		1.5%
- BasicCCC                                10.8ms	10.5ms	261µs		2.5%
- CaseInsensitiveCCC                      12ms	11.7ms	255µs		2.2%
- IntersectionCCC                         22.1ms	21.9ms	206µs		0.9%
- BasicRangeCCC                           11.1ms	11ms	196µs		1.8%
- IPv4Address                             2.64ms	2.54ms	99.2µs		3.9%
=== Improvements =====================================================================
- CompilerMessagesAll                     113ms	116ms	-2.71ms		-2.3%
- DiceRollsInTextAll                      47.3ms	49.6ms	-2.23ms		-4.5%
- AnchoredNotFoundWhole                   7.44ms	9.18ms	-1.74ms		-18.9%
- NotFoundAll                             6.29ms	7.09ms	-797µs		-11.2%
- EmailLookaheadAll                       39.3ms	39.9ms	-683µs		-1.7%
- EmailBuiltinCharacterClassAll           14.8ms	15.5ms	-663µs		-4.3%
- ReluctantQuantWithTerminalWhole         8.77ms	9.3ms	-525µs		-5.6%
- LiteralSearchAll                        6.17ms	6.65ms	-478µs		-7.2%
- HangulSyllableAll                       6.35ms	6.76ms	-412µs		-6.1%
- LiteralSearchNotFoundAll                6.05ms	6.42ms	-369µs		-5.8%
- GraphemeBreakNoCapAll                   5.26ms	5.52ms	-264µs		-4.8%
- WordsAll                                14.2ms	14.5ms	-264µs		-1.8%
- symDiffCCC                              48.7ms	49ms	-240µs		-0.5%
- EmailLookaheadList                      9.65ms	9.88ms	-230µs		-2.3%
- CssAll                                  3.63ms	3.85ms	-222µs		-5.8%
- EmailLookaheadNoMatchesAll              40.6ms	40.8ms	-204µs		-0.5%
- HangulSyllableFirst                     3.06ms	3.25ms	-194µs		-6.0%
- DiceNotation                            5.21ms	5.39ms	-179µs		-3.3%
- BasicBuiltinCharacterClassAll           9.02ms	9.13ms	-108µs		-1.2%
- EagarQuantWithTerminalWhole             2.55ms	2.61ms	-62.8µs		-2.4%
- LinesAll                                3.05ms	3.1ms	-42.7µs		-1.4%
- MACAddress                              3ms	3.02ms	-24.7µs		-0.8%

@milseman milseman requested a review from natecook1000 April 3, 2023 17:29
// return returnValue
// }
//
// }
Copy link
Member Author

Choose a reason for hiding this comment

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

TODO: investigate re-enabling this code, or even consider looking at all potential callers of loadScalar.

return nextByteIdx
}
}
return nil
Copy link
Member Author

Choose a reason for hiding this comment

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

TODO: add assertion to check fast-path produces same result as slow path

}
return nil
} // 13-22ms, after: 22-25ms ???
// Now down to 3ms
Copy link
Member Author

Choose a reason for hiding this comment

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

TODO: remove

@stephentyrone
Copy link
Contributor

Not sure why that got marked as a review. Ignore.

@stephentyrone stephentyrone self-requested a review April 3, 2023 17:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants