Smol String #14755
Merged
Smol String #14755
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@swift-ci please test (I've been passing clean locally on release+asserts, but not debug+asserts) |
|
(expecting regressions) @swift-ci please smoke benchmark |
|
@swift-ci please test |
|
@swift-ci please smoke benchmark |
|
Build failed |
|
Build failed |
Build comment file:Optimized (O)Regression (37)
Improvement (16)
No Changes (325)
Unoptimized (Onone)Regression (29)
Improvement (14)
No Changes (335)
Hardware Overview |
|
Regression from |
|
So yeah, that's not nearly as bad as I feared and I think a lot of it will improve when we switch to a more sane bit-layout and branching scheme. |
|
Now trying with ASCII literals @swift-ci please smoke benchmark |
|
(expected net regression from spill-based approaches, just interested in magnitude) |
Build comment file:Optimized (O)Regression (76)
Improvement (19)
No Changes (283)
Unoptimized (Onone)Regression (62)
Improvement (36)
No Changes (280)
Hardware Overview |
|
Woo, there's the regressions I was expecting! |
|
@swift-ci please smoke benchmark |
|
StringEnumRawValueInitialization is still slower, but it highlights the discrepancy between static strings (which can now be small) and dynamic ones (which haven't been hooked up yet). I want to reduce these regressions before they're lost in the noise of dynamic small string. |
Build comment file:Optimized (O)Regression (84)
Improvement (25)
No Changes (269)
Unoptimized (Onone)Regression (64)
Improvement (24)
No Changes (290)
Hardware Overview |
|
(There's some kind of bug that I haven't hunted down yet, so that could also influence benchmark behavior). @swift-ci please smoke benchmark |
Build comment file:Optimized (O)Regression (75)
Improvement (18)
No Changes (285)
Unoptimized (Onone)Regression (89)
Improvement (26)
No Changes (263)
Hardware Overview |
|
@airspeedswift was replacing |
|
@swift-ci please smoke benchmark |
Build comment file:Optimized (O)Regression (64)
Improvement (22)
No Changes (292)
Unoptimized (Onone)Regression (71)
Improvement (35)
No Changes (272)
Hardware Overview |
|
(added a few sources of dynamic small strings, but haven't done append/join/etc) @swift-ci please smoke benchmark |
Build comment file:Optimized (O)Regression (60)
Improvement (41)
No Changes (277)
Unoptimized (Onone)Regression (59)
Improvement (37)
No Changes (282)
Hardware Overview |
|
Loads more dynamic small strings. Implementation approach for index-operations is still spill-and-go. Missing some appends still... @swift-ci please smoke benchmark |
|
@swift-ci please smoke benchmark |
Streamline internal String creation. Previously, everything funneled into a single generic function, however, every single call of the generic funnel had relevant specific information that could be used for a more efficient algorithm. In preparation for efficiently forming small strings, refactor this logic into a handful of more specialized subroutines to preserve more specific information from the callers.
|
Build failed |
This adds a small string representation capable of holding up to 15 ASCII code units directly in registers. This is extendable to UTF-8 in the future. It is intended to be the preferred representation whenever possible for String, and is intended to be a String fast-path. Future small forms may be added in the future (likely off the fast-path). Small strings are available on 64-bit, where they are most beneficial and well accomodated by limited address spaces. They are unavailable on 32-bit, where they are less of a win and would require much more hackery due to full address spaces.
Switch StringObject and StringGuts from opaquely storing tagged cocoa strings into storing small strings. Plumb small string support throughout the standard library's routines.
Whenever possible, prefer the small string format. Updates creation logic. Adds assertion on large strings that they're not intended to be small.
|
Please test with the following PR: apple/swift-lldb#461 @swift-ci please test Linux Platform |
|
Build failed |
|
Please test with the following PR: apple/swift-lldb#461 @swift-ci please test OS X Platform |
|
Build failed |
|
@milseman I would just use a requires line and continue. |
|
@dcci @vedantk so I've verified that my formatting code does handle intermediary nul characters correctly. Do you know why this test might fail? https://ci.swift.org/job/swift-PR-osx/3925/consoleFull#376610391ba62d58e-7248-467b-91e0-c7508d5cf947 When I tried to run LLDB tests locally, I could get all the LIT based ones working but the python ones all kept crashing (likely I had a mismatched version of Python). Are they supposed to work with a CMake based build? |
|
@milseman The cmake build of lldb should support running all the python tests. Separately, I can follow up and see why things are failing for you locally. I'll take a look at the test now. |
|
@milseman It looks like apple/swift-lldb#461 forwards intermediary nul characters directly to the formatter's output string without escaping them first. I've left a more detailed comment + suggested fix in the PR. |
|
Please test with the following PR: apple/swift-lldb#461 @swift-ci please test OS X Platform |
|
Please test with the following PR: apple/swift-lldb#461 @swift-ci please test Linux Platform |
|
Build failed |
|
Please test with the following PR: @swift-ci please test Linux Platform |
|
Build failed |
|
Please test with the following PR: @swift-ci please test Linux Platform |
|
Congrats Michael! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add in small strings, plumb all APIs through, etc. There's some current performance issues, namely that small string support introduces branching and our default implementation technique involves spilling to the stack and calling
_UnmanagedStringmethods. The first is unavoidable (although we're planning a better overall branching strategy to reduce this impact), the second is TODO.