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.

Sign up for GitHub

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

Jump to bottom

Critical String fixes and high-value enhancements #9784

Merged
merged 6 commits into from
May 20, 2017
Merged

Critical String fixes and high-value enhancements #9784

merged 6 commits into from
May 20, 2017

Conversation

dabrahams
Copy link
Contributor

@dabrahams dabrahams commented May 19, 2017 •
edited
Loading

Explanation: Critical String fixes and high-value enhancements
Scope: security bug fix, source-compatible library extension, optimizations
Radar: rdar://32300763 Critical String fixes and high-value enhancements
Risk: moderate
Testing: Passed full validation + benchmarking on master; passed validation and and source compatibility on the 4.0 branch; benchmark pending.

@dabrahams
Copy link
Contributor Author

@swift-ci Please test

@dabrahams
Copy link
Contributor Author

@swift-ci Please test source compatibility

@dabrahams
Copy link
Contributor Author

@swift-ci Please benchmark

Dave Abrahams added 6 commits May 19, 2017 11:56
[stdlib] Add initial Substring benchmarks
8b65bd1
[stdlib] Provisional c[...] syntax while we await language features
0e21be1
[stdlib] A Character's buffer should only contain 1 grapheme
e7f890a 
This speeds up construction of a String from large Character representations,
and various other operations that would otherwise require additional grapheme
breaking just to interpret the Character.
[stdlib] Enable interchange among StringProtocol models
fcc93cd
[stdlib] Replace Integer Parsing Code
c1ec4a0
[stdlib] Fix a race
60d6e8b 
Lock-free programming is almost always a bug.
Fixes <rdar://25398370> Data Race in StringBuffer.append (found by TSan)
@dabrahams
Copy link
Contributor Author

@swift-ci Please test

1 similar comment
@dabrahams
Copy link
Contributor Author

@swift-ci Please test

@dabrahams
Copy link
Contributor Author

@swift-ci Please benchmark

@dabrahams
Copy link
Contributor Author

@swift-ci Please test source compatibility

@dabrahams dabrahams changed the title String updates Critical String fixes and high-value enhancements May 19, 2017
@swift-ci
Copy link
Contributor

Build comment file:

Build failed before running benchmark.

@tkremenek
Copy link
Member

@airspeedswift @dabrahams Benchmarking results are available if you go to the details of the job. The job failed because it expects to get results back for master. Do the results look like what you expect?

@dabrahams
Copy link
Contributor Author

AFAICT when I got to the details I don't get anything I can compare to. I see one set of results for O and O_none. I'm not even sure these are the results for the branch under test, as AFAIK Mishal has the baseline tests run first.

@tkremenek tkremenek merged commit dbed002 into swift-4.0-branch May 20, 2017
@tkremenek tkremenek mentioned this pull request May 21, 2017
Merged
@tkremenek
Copy link
Member

This was reverted in:

#9813

pending investigation if SR-4955 is related to this change.

@dabrahams
Copy link
Contributor Author

Given my current understanding of the problem, I'd say the risk here is quite low, FWIW. I'll keep the bug updated, though.

@tkremenek tkremenek mentioned this pull request May 21, 2017
Merged
@jrose-apple jrose-apple deleted the string-beta-1 branch November 29, 2017 22:35
@tkremenek tkremenek mentioned this pull request May 21, 2017
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@airspeedswift airspeedswift Awaiting requested review from airspeedswift

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@dabrahams @swift-ci @tkremenek