Skip to content

[SyntaxModel] Improve the performance of searching URLs in comments #5214

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 4 commits into from
Oct 10, 2016
Merged

[SyntaxModel] Improve the performance of searching URLs in comments #5214

merged 4 commits into from
Oct 10, 2016

Conversation

nkcsgexi
Copy link
Contributor

@nkcsgexi nkcsgexi commented Oct 10, 2016

[SyntaxModel] When searching URLs in doc comments, reduce the number of protocol name comparisons by looking ahead more characters, NFC. rdar://28298506

Searching URL in doc comments can be expensive. We used to look for
every colon as an indicator of potential URLs. However, this is not
efficient enough. Suggested by Ben, we further divide protocols into
categories so that most protocols can use "://" as an indicator of its
existence.

Not sure whether this is enough to close the radar, but I believe it is
a valuable performance improvement anyway.

rdar://problem/28705426

…tions.

Keywords like 'let' can serve as argument labels. When they do so, we should
highlight them as identifiers instead of keywords. However, the check for this
situation seems overly lenient so that when 'let', 'var' appear in conditions
of IfStmt or GuardStmt, they are wrongly highlighted as identifiers too. This
commit strengthens the checking to preserve keywords' identity in these statements.
protocol name comparisons by looking ahead more characters, NFC. rdar://28298506

Searching URL in doc comments can be expensive. We used to look for
every colon as an indicator of potential URLs. However, this is not
efficient enough. Suggested by Ben, we further divide protocols into
categories so that most protocols can use "://" as an indicator of its
existence.

Not sure whether this is enough to close the radar, but I believe it is
a valuable performance improvement anyway.
@nkcsgexi
Copy link
Contributor Author

@swift-ci smoke test

@nkcsgexi
Copy link
Contributor Author

@swift-ci smoke test

1 similar comment
@nkcsgexi
Copy link
Contributor Author

@swift-ci smoke test

@nkcsgexi nkcsgexi merged commit 4d946f4 into swiftlang:master Oct 10, 2016
MaxDesiatov pushed a commit that referenced this pull request Sep 7, 2023
[pull] swiftwasm from main
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.

1 participant