Skip to content

Conversation

@ahoppen
Copy link
Member

@ahoppen ahoppen commented Mar 21, 2023

This replaces assert statements by precondition statements in most cases. Afterwards I did a few performance measurements and reverted a few preconditions that had significant performance impact back to assert statements.

As it stands now, this regresses performance of parsing MovieSwiftUI by 2.9% (measured in number of instructions executed).

The biggest (>0.1%) performance regressions are coming from the following files. I’m open to debate if any of the preconditions in there should be changed to assert to improve performance but they all seem to provide good value at a reasonable cost to me

0.62% Sources/SwiftParser/TokenConsumer.swift
0.58% Sources/SwiftSyntax/Raw/RawSyntax.swift
0.52% Sources/SwiftParser/Lexer/Cursor.swift
0.41% Sources/SwiftSyntax/SyntaxText.swift
0.39% Sources/SwiftParser/TokenSpec.swift
0.29% Sources/SwiftParser/Recovery.swift
0.26% Sources/SwiftParser/Names.swift

I am also attaching the numbers spreadsheet that contains all performance measurements: Precondition performance measurements.zip

rdar://106874489

@ahoppen ahoppen changed the title Replace assert by precondition in most places 🚥 #1342 Replace assert by precondition in most places Mar 23, 2023
@ahoppen ahoppen force-pushed the ahoppen/precondition branch from d4b6704 to 647fb80 Compare March 23, 2023 23:28
@ahoppen ahoppen force-pushed the ahoppen/precondition branch from 647fb80 to eaefea6 Compare March 23, 2023 23:28
@ahoppen
Copy link
Member Author

ahoppen commented Mar 23, 2023

@swift-ci Please test

@ahoppen ahoppen merged commit 3bb2659 into swiftlang:main Mar 24, 2023
@ahoppen ahoppen deleted the ahoppen/precondition branch March 24, 2023 14:17
ahoppen added a commit to ahoppen/swift-syntax that referenced this pull request Mar 31, 2023
Replace `assert` by `precondition` in most places
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