mount/mountinfo_linux: parser speed up #2324
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@kolyshkin ci failed with |
The mountinfo parser implemented via `fmt.Sscanf()` is slower than the one using `strings.Split()` and `strconv.Atoi()`. This rewrite helps to speed it up to a factor of 8x, here is a result from `go bench`: > BenchmarkParsingScanf-4 300 22294112 ns/op > BenchmarkParsingSplit-4 3000 2780703 ns/op I tried other approaches, such as using `fmt.Sscanf()` for the first three (integer) fields and `strings.Split()` for the rest, but it slows things down considerably: > BenchmarkParsingMixed-4 1000 8827058 ns/op Note the old code uses `fmt.Sscanf` first, then a linear search for the '-' field, then a split for the last 3 fields. The new code relies on a single split. One other thing is, the new code is more future proof as it skips extra optional fields before the separator (currently there are none). I have also added more comments to aid in future development. Finally, the test data is fixed to not have white space before the first field. Based on a similar change in Moby, moby/moby#36091 [v2: remove no-op break statement to silence staticcheck] Signed-off-by: Kir Kolyshkin <[email protected]>
|
Apparently I had an old version of gometalinter locally. Fixed. |
Codecov Report
@@ Coverage Diff @@
## master #2324 +/- ##
==========================================
- Coverage 44.98% 44.98% -0.01%
==========================================
Files 92 92
Lines 9331 9340 +9
==========================================
+ Hits 4198 4202 +4
- Misses 4456 4459 +3
- Partials 677 679 +2
Continue to review full report at Codecov.
|
|
LGTM |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The mountinfo parser implemented via
fmt.Sscanf()is slower than the one usingstrings.Split()andstrconv.Atoi(). This rewrite helps to speed it up to a factor of 8x, here is a result fromgo bench:I tried other approaches, such as using
fmt.Sscanf()for the first three (integer) fields andstrings.Split()for the rest, but it still slows things down considerably:Note the old code uses
fmt.Sscanffirst, then a linear search for '-' field, then a split for the last 3 fields. The new code relies on a single split.One other thing is, the new code is more future proof as it skips extra optional fields before the separator (currently there are none).
I have also added more comments to aid in future development.
Finally, the test data is fixed to not have white space before the first field.
Based on a similar change in Moby, moby/moby#36091.