Overhaul quantification optimizations #710

milseman · 2023-12-15T17:41:41Z

Effectively one code implementation for all optimized (non-reluctant) quantifications
Specializations via inlining
Non-mutating methods on String (easier to test, optimize, and could be future API fodder)
Set up for further improvements

Supersedes #706

Refactors mutating methods into string methods for easier unit testing and parity-checking via assertions. Prepares for more efficient implementations.

milseman · 2023-12-15T18:35:25Z

=== Regressions ======================================================================
- CompilerMessages_All                    92.4ms	91.5ms	842µs		0.9%
- CompilerMessages_All_Scalar             75.6ms	74.8ms	815µs		1.1%
- BasicCCC_All_Scalar                     6.35ms	6.02ms	331µs		5.5%
- BasicCCC_All                            6.35ms	6.03ms	324µs		5.4%
- BasicRangeCCC_All_Scalar                6.8ms	6.5ms	301µs		4.6%
- CaseInsensitiveCCC_All                  6.75ms	6.46ms	297µs		4.6%
- BasicRangeCCC_All                       6.8ms	6.5ms	296µs		4.5%
- CaseInsensitiveCCC_All_Scalar           6.75ms	6.46ms	289µs		4.5%
- BasicBuiltinCharacterClass_All_Scalar   7.44ms	7.24ms	200µs		2.8%
- ReluctantQuant_Whole_Scalar             9.31ms	9.16ms	153µs		1.7%
- Lines_All                               1.79ms	1.74ms	53µs		3.0%
- Lines_All_Scalar                        1.75ms	1.7ms	50.3µs		3.0%
- MACAddress                              2.43ms	2.4ms	35.2µs		1.5%
- MACAddress_Scalar                       2.3ms	2.27ms	23µs		1.0%
- IPv4Address_Scalar                      2.07ms	2.06ms	14.1µs		0.7%
=== Improvements =====================================================================
- symDiffCCC_All                          56.9ms	58.2ms	-1.28ms		-2.2%
- symDiffCCC_All_Scalar                   56.9ms	58.2ms	-1.26ms		-2.2%
- SubtractionCCC_All_Scalar               20.9ms	22ms	-1.16ms		-5.3%
- IntersectionCCC_All_Scalar              21.8ms	22.9ms	-1.09ms		-4.8%
- SubtractionCCC_All                      20.9ms	21.9ms	-1.07ms		-4.9%
- IntersectionCCC_All                     21.8ms	22.8ms	-1.05ms		-4.6%
- EmailLookahead_All                      18.4ms	19.4ms	-1.02ms		-5.3%
- EmailLookahead_All_Scalar               16.2ms	17.2ms	-1ms		-5.8%
- AnchoredNotFound_All                    13.6ms	14.5ms	-909µs		-6.3%
- DiceRollsInText_All                     41ms	41.8ms	-746µs		-1.8%
- AnchoredNotFound_All_Scalar             8.96ms	9.67ms	-706µs		-7.3%
- AnchoredNotFound_First_Scalar           5.32ms	5.98ms	-656µs		-11.0%
- DiceRollsInText_All_Scalar              38.6ms	39.2ms	-652µs		-1.7%
- EmailRFC_All                            63.3ms	63.8ms	-478µs		-0.7%
- EagarQuantWithTerminal_Whole            441µs	779µs	-338µs		-43.4%
- EagarQuantWithTerminal_Whole_Scalar     442µs	779µs	-338µs		-43.3%
- EmailLookaheadList                      3.62ms	3.91ms	-293µs		-7.5%
- EmailLookaheadList_Scalar               3.39ms	3.68ms	-290µs		-7.9%
- InvertedCCC_All_Scalar                  16.6ms	16.8ms	-241µs		-1.4%
- AnchoredNotFound_First                  9.61ms	9.79ms	-182µs		-1.9%
- GraphemeBreakNoCap_All_Scalar           2.93ms	3.1ms	-174µs		-5.6%
- NotFound_All                            7.15ms	7.31ms	-160µs		-2.2%
- NotFound_All_Scalar                     6.17ms	6.33ms	-156µs		-2.5%
- LiteralSearch_All_Scalar                5.69ms	5.84ms	-154µs		-2.6%
- GraphemeBreakNoCap_All                  3.07ms	3.21ms	-144µs		-4.5%
- InvertedCCC_All                         16.6ms	16.7ms	-144µs		-0.9%
- BasicBuiltinCharacterClass_All          8.05ms	8.19ms	-141µs		-1.7%
- LiteralSearchNotFound_All_Scalar        5.5ms	5.64ms	-137µs		-2.4%
- LiteralSearch_All                       6.65ms	6.79ms	-133µs		-2.0%
- Words_All_Scalar                        12ms	12.2ms	-130µs		-1.1%
- EmailLookaheadNoMatches_All             22.8ms	22.9ms	-123µs		-0.5%
- IPv6Address_Scalar                      2.29ms	2.4ms	-110µs		-4.6%
- Words_All                               12.5ms	12.6ms	-108µs		-0.9%
- IPv6Address                             2.48ms	2.57ms	-92µs		-3.6%
- LiteralSearchNotFound_All               6.5ms	6.59ms	-90.1µs		-1.4%
- DiceNotation                            4.47ms	4.55ms	-80.5µs		-1.8%
- EmojiRegex_All_Scalar                   40.4ms	40.5ms	-75.8µs		-0.2%
- DiceNotation_Scalar                     4.23ms	4.3ms	-66.9µs		-1.6%
- Numbers_All_Scalar                      5.21ms	5.27ms	-61.6µs		-1.2%
- HangulSyllable_All_Scalar               5.97ms	6.03ms	-54.1µs		-0.9%
- Numbers_All                             5.94ms	5.99ms	-51.9µs		-0.9%
- HangulSyllable_All                      6.71ms	6.77ms	-50.5µs		-0.7%
- Css_All                                 3.23ms	3.27ms	-39.3µs		-1.2%
- HangulSyllable_First_Scalar             2.89ms	2.93ms	-30.1µs		-1.0%
- Css_All_Scalar                          2.88ms	2.91ms	-29.6µs		-1.0%
- EmailBuiltinCharacterClass_All          10.3ms	10.3ms	-18.3µs		-0.2%

milseman · 2023-12-15T18:35:51Z

@swift-ci please test

natecook1000

Just a few questions, otherwise looks good!

natecook1000 · 2023-12-18T15:26:01Z

Sources/_StringProcessing/ConsumerInterface.swift

@@ -603,9 +602,9 @@ extension AST.Atom.CharacterProperty {
        if p(input, bounds) != nil { return nil }

        // TODO: bounds check


What's the story here — does the bounds check always happen before the consume function is called? Or is there a post-advancement check to make sure we haven't gone beyond the bounds of the search range?

This doesn't change the behavior. Let's open an issue for that

natecook1000 · 2023-12-18T15:44:32Z

Sources/_StringProcessing/Engine/MEBuiltins.swift

+    limitedBy end: Index,
+    anyMatchesNewline: Bool,
+    isScalarSemantics: Bool
+  ) -> Index? {


Are we still making distinctions between the . literal syntax and other "any" designations at this point in the engine? For example, CharacterClass.any and CharacterClass.anyNonNewline are unaffected by the (?s) option, and always do what they say they should. I thought that we had already handled those distinctions during code gen (e.g. by calling emitAny vs emitAnyNonNewline).

If this method is just for the . literal, does it need to account for the NSRE-compatible-dot mode? In that case, a full \r\n pair needs to be consumed even when using scalar semantics.

This is the leaf function which provides the implementation. The anyMatchesNewline parameter dictates what the behavior is, and is at least part of the quantification payload.

natecook1000 · 2023-12-18T15:48:42Z

Sources/_StringProcessing/Engine/MEQuantify.swift

+      signalFailure()
+      return false
+    }
+    if let savePointRange {


assert(!produceSavePointRange) here would make sure we don't accidentally return a save point range when instructed not to.

@Catfish-Man could you take over and see about that assertion?

Once I figure out my build issues, yup :)

natecook1000 · 2023-12-18T16:07:55Z

Sources/_StringProcessing/Engine/MEQuantify.swift


-    while true {
-      if maxExtraTrips == 0 { break }
+    return (currentPosition, rangeStart..<rangeEnd)


With the assertion above, should this be Range(uncheckedBounds:) instead of ..<?

Yes, that would work.

milseman added 14 commits December 11, 2023 15:54

Speed up quantification fast paths by unswitching the loop

c2b93ab

squash me: cleanup

b48f09e

comments

efd4a1d

Refactor off of mutating methods

45b4da3

Refactors mutating methods into string methods for easier unit testing and parity-checking via assertions. Prepares for more efficient implementations.

wip: trying to generalize and make fast enough

eb6e0e0

wip: on the way to total un-switching while converting to non-mutating

3fe6fe2

wip: getting closer

1687315

wip

15706a4

wip

01177fa

all converted to string methods

e5ee845

wip

2d7a391

fix findo/replaco

4c3ebf0

better names and comments

3c5dea0

wip: typo

50b2f91

milseman requested review from stephentyrone and natecook1000 December 15, 2023 17:41

natecook1000 reviewed Dec 18, 2023

View reviewed changes

milseman assigned Catfish-Man Jan 31, 2024

Catfish-Man mentioned this pull request Feb 3, 2024

Overhaul quantification optimizations #716

Merged

Catfish-Man merged commit 50b2f91 into swiftlang:main Feb 7, 2024

milseman deleted the overhual_builtin_quant branch February 7, 2024 22:31

milseman mentioned this pull request May 31, 2024

Update release 6.0 with main #743

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Overhaul quantification optimizations #710

Overhaul quantification optimizations #710

Uh oh!

milseman commented Dec 15, 2023

Uh oh!

milseman commented Dec 15, 2023

Uh oh!

milseman commented Dec 15, 2023

Uh oh!

natecook1000 left a comment

Uh oh!

natecook1000 Dec 18, 2023

Uh oh!

milseman Jan 31, 2024

Uh oh!

natecook1000 Dec 18, 2023

Uh oh!

milseman Jan 31, 2024

Uh oh!

natecook1000 Dec 18, 2023

Uh oh!

milseman Jan 31, 2024

Uh oh!

Catfish-Man Feb 1, 2024

Uh oh!

natecook1000 Dec 18, 2023

Uh oh!

milseman Jan 31, 2024

Uh oh!

Uh oh!

		@@ -603,9 +602,9 @@ extension AST.Atom.CharacterProperty {
		if p(input, bounds) != nil { return nil }

		// TODO: bounds check

Overhaul quantification optimizations #710

Overhaul quantification optimizations #710

Uh oh!

Conversation

milseman commented Dec 15, 2023

Uh oh!

milseman commented Dec 15, 2023

Uh oh!

milseman commented Dec 15, 2023

Uh oh!

natecook1000 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!