Skip to content

Conversation

@Catfish-Man
Copy link
Contributor

@Catfish-Man Catfish-Man commented Jul 29, 2025

Same as #83073, but with an additional optimization I wanted to try in parallel

Fixes rdar://141789595

@Catfish-Man Catfish-Man self-assigned this Jul 29, 2025
@Catfish-Man
Copy link
Contributor Author

@swift-ci please smoke test

@Catfish-Man
Copy link
Contributor Author

@swift-ci please Apple Silicon benchmark

@Catfish-Man
Copy link
Contributor Author

@swift-ci please benchmark

@Catfish-Man
Copy link
Contributor Author

@swift-ci please smoke test

@Catfish-Man
Copy link
Contributor Author

@swift-ci please smoke test

@Catfish-Man
Copy link
Contributor Author

@swift-ci please smoke test

@Catfish-Man Catfish-Man marked this pull request as ready for review July 30, 2025 18:08
@Catfish-Man Catfish-Man requested a review from a team as a code owner July 30, 2025 18:08
@Catfish-Man
Copy link
Contributor Author

@swift-ci please Apple Silicon benchmark

@Catfish-Man
Copy link
Contributor Author

Just to double check that I didn't break perf when I switched away from InlineArray :)

@_transparent func withLengthLUT(
_ work: (UnsafeRawBufferPointer) -> Int
) -> Int {
let lut: (
Copy link
Contributor

Choose a reason for hiding this comment

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

We can probably use an InlineArray<128, UInt8> here.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh I didn't read your latest message that said you came from an InlineArray. Any reason you switched away from it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Availability issues :/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But this approach is actually all wrong anyway, and only faster because the codegen for our SIMD types has issues. Steve says he’s going to fix it, then I’ll retest and likely delete the table entirely.

Values >= 0x4000 are handled via the scalar path to keep the table size down
*/
@_transparent func withLengthLUT(
_ work: (UnsafeRawBufferPointer) -> Int
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we start using spans here?

repairing: Bool = true
) -> (String, repairsMade: Bool)? {
if input.isEmpty { return ("", repairsMade: false) }
guard let (utf8Len, isASCII) = unsafe utf8Length(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Once we're ready to push forward on this again, a test I just ran locally suggests that adding a callback to resize the allocation if needed (guess all-ascii, then resize by 2x, then by 1.5x) is actually quite a bit faster than sizing up front.

We'll see if I can make that work for String, will require a bit of shenanigans probably.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fun option: port String growth-on-append to realloc then use that

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