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

Impromptu perf fix: NSString _fastestEncoding #1623

Merged
merged 1 commit into from
Jan 4, 2017

Conversation

ms-jihua
Copy link
Contributor

@ms-jihua ms-jihua commented Jan 4, 2017

Found while checking CTCatalog affine transform test.
In cases of longer strings, _DWriteGetFrame() spends an inordinate amount of time in:
_DWriteGetFrame (~30%) -> [NSString substring...] (~27%) -> [NSString _fastestEncodingWith...] (~25%) (sample trace)

Changed this function to not call [self length] every iteration, fetch characters in chunks.
After this change, the same test looks like:
_DWriteGetFrame (~5%) -> [NSString substring...] (~1.2%) -> [NSString _fastestEncodingWith...] (~0.5%) (sample trace)


This change is Reviewable

options:(NSStringEncodingConversionOptions)options
range:(NSRange)range
remainingRange:(NSRangePointer)left {
maxLength:(NSUInteger)maxBuf
Copy link
Contributor Author

Choose a reason for hiding this comment

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

looks like our clang-format got updated (10/31) since the last change to this file (early sep)

NSRange range;

for (range.location = 0; range.location < length; range.location += range.length) {
UniChar buf[bufferSize];
Copy link
Contributor

Choose a reason for hiding this comment

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

UniChar buf[bufferSize] [](start = 8, length = 23)

move this buffer outside the for loop.

for (int32_t i = 0; i < [self length]; i++) {
if ([self characterAtIndex:i] > 0x7F) {
// Check characters in chunks for perf reasons
const size_t bufferSize = 128;
Copy link
Contributor

Choose a reason for hiding this comment

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

128 [](start = 30, length = 3)

consider making this bigger (512?).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure

Copy link
Contributor

@rajsesh rajsesh left a comment

Choose a reason for hiding this comment

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

Could you please update the commit to refer #1621?

@rajsesh
Copy link
Contributor

rajsesh commented Jan 4, 2017

:shipit:

Found while checking CTCatalog affine transform test.
In cases of longer strings, _DWriteGetFrame() spends an inordinate amount of time in:
_DWriteGetFrame (~30%) -> [NSString substring...] (~27%) -> [NSString _fastestEncodingWith...] (~25%)  (sample trace)

Changed this function to not call [self length] every iteration, fetch characters in chunks.
After this change, the same test looks like:
_DWriteGetFrame (~5%) -> [NSString substring...] (~1.2%) -> [NSString _fastestEncodingWith...] (~0.5%)  (sample trace)

Related to microsoft#1621
@ms-jihua ms-jihua merged commit bf9b2f8 into microsoft:develop Jan 4, 2017
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.

3 participants