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

Speedup Xcbeautify by applying frequency-based pattern matching. #67

Merged
merged 3 commits into from Feb 28, 2022

Conversation

PaulTaykalo
Copy link
Contributor

@PaulTaykalo PaulTaykalo commented Feb 24, 2022

The current implementation of xcbeautify performs regex checking for each line of the xcodebuild output.
Due to the big amount of possible regexes, each line on average is checked for about N/2 times. which is a lot

Suggested implementation holds an array of 'inner parser' and try to reuse 'line parser/matchers' using frequency based array (basically, last matcher regex will be checked first).
Due to the non-random structure of xcodebuil output, this approach allows to decrease xcbeautify running time from 160 s to 9 seconds when used on the xcodebuild output of size 240Mb

Implementation time Log Size
xcbeautify-main 209s 256M
xcbeautify-speedup 8s 256M
xcpretty 23s 256M

Testing approach:
time cat LOGFILE | xcbeauitify

Possible 'bugs'

If there are multiple regexes which can match same line, this approach could return different results, depending on the order of the commands, appeared in log

Xcbeautify Speed

It seems that xcbeautify is waaaay slower than xcpretty. This PR will actually make xcbeautify faster


private let colored: Bool

private let additionalLines: () -> String?
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved parameters to the Parser itself out of the method, which should return beautified string


private let colored: Bool

private let additionalLines: () -> String?
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved parameters to the Parser itself out of the method, which should return beautified string

@PaulTaykalo PaulTaykalo changed the title Feature/speedup xcbeautify Speedup Xcbeautify by applying frequency-based pattern matching. Feb 24, 2022
@@ -41,7 +41,7 @@ enum Pattern: String {

/// Regular expression captured groups:
/// $1 = file
case codesign = #"CodeSign\s((?:\ |[^ ])*)$"#
case codesign = #"CodeSign\s(((?!.framework/Versions/A)(?:\ |[^ ]))*)$"#
Copy link
Contributor Author

Choose a reason for hiding this comment

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

since now, whichever patter matches first is winning, we need to make sure that codesign pattern will not 'eat' codesignFramework pattern

@PaulTaykalo
Copy link
Contributor Author

@thii ^^

Copy link
Contributor

@thii thii left a comment

Choose a reason for hiding this comment

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

I really don't remember much of everything here, but LGTM.

Sources/XcbeautifyLib/Parser.swift Outdated Show resolved Hide resolved
Tests/XcbeautifyLibTests/XcbeautifyLibTests.swift Outdated Show resolved Hide resolved
@PaulTaykalo
Copy link
Contributor Author

Fixed suggestions.

@thii
Copy link
Contributor

thii commented Feb 25, 2022

Sorry I just merged a PR that introduced some conflicts here. Would you be able to fix them?

@PaulTaykalo
Copy link
Contributor Author

Yep. Sure

Add tests for some inconsistent parsing of signing

Fix cases with signing ordering

Update Sources/XcbeautifyLib/Parser.swift

Co-authored-by: Thi Doãn <t@thi.im>

Update Tests/XcbeautifyLibTests/XcbeautifyLibTests.swift

Co-authored-by: Thi Doãn <t@thi.im>
@PaulTaykalo
Copy link
Contributor Author

@thii done

@@ -12,6 +12,7 @@ class Regex {
}

func match(string: String) -> Bool {
return matcher?.numberOfMatches(in: string, range: NSRange(location: 0, length: string.count)) != 0
let fullRange = NSRange(string.startIndex..., in: string)
return matcher?.rangeOfFirstMatch(in: string, range: fullRange).location != NSNotFound
Copy link
Contributor Author

Choose a reason for hiding this comment

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

even faster variant, with correct range calculation

@thii thii merged commit e52d61b into cpisciotta:main Feb 28, 2022
@thii
Copy link
Contributor

thii commented Feb 28, 2022

Thanks @PaulTaykalo!

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.

None yet

2 participants