Skip to content

[Optimization] Add instructions for consuming non-newlines and advancing in scalar view #596

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

Merged
merged 1 commit into from
Aug 10, 2022

Conversation

rctcwyvrn
Copy link
Contributor

A simple new instruction to replace the consumers that are used for emitting .any and .anyNonNewline instructions in scalar and grapheme mode

Makes matching a default options . about ~3-5% faster

- ReluctantQuantWithTerminalWhole         7.8ms	8.2ms	-402µs		-4.9%
- ReluctantQuantWhole                     11.7ms	12ms	-345µs		-2.9%

Doesn't affect a lot of benchmarks in our suite because we mostly have quantified .s like .* and that ends up in the .quantify optimization
image

@rctcwyvrn rctcwyvrn requested a review from milseman August 3, 2022 23:36
@milseman
Copy link
Member

milseman commented Aug 4, 2022

Is this substantially different than a consume builtin where anyNonNewline is a kind of builtin? I.e. is the dedicated opcode just a particular encoding of the operation?

@rctcwyvrn
Copy link
Contributor Author

We could write it like this

mutating func emitAnyNonNewline() {
    let model = _CharacterClassModel.init(
      cc: .verticalWhitespace,
      options: options,
      isInverted: true)
    builder.buildMatchBuiltin(model: model)
  }

But since this falls into the switch statement with all the other builtin cases it ends up being much slower

=== Regressions ======================================================================
- ReluctantQuantWhole                     14ms	12ms	2.01ms		16.7%
- ReluctantQuantWithTerminalWhole         10.1ms	8.2ms	1.88ms		22.9%

@milseman
Copy link
Member

Right, so the choice of a different instruction is reorganizing the branching story. There are lots of ways to organize this, e.g. we could branch early for this case or we could even switch over the combination of op-code and builtin (and for non-builtin-taking operations, that would be zero-ed out).

Anyways, I'm fine taking this because it locks in the kind of performance story we will want at the end.

@rctcwyvrn
Copy link
Contributor Author

@swift-ci test

@rctcwyvrn rctcwyvrn merged commit ceecaaa into swiftlang:main Aug 10, 2022
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