Skip to content

Conversation

IncSW
Copy link
Contributor

@IncSW IncSW commented May 3, 2021

go version go1.16 linux/amd64

BenchmarkSkipWhiteSpace/gotoLabel-8   25363138   47.65 ns/op
BenchmarkSkipWhiteSpace/loop-8        25359750   47.43 ns/op

goto is not needed inside skipWhiteSpace

@goccy
Copy link
Owner

goccy commented May 3, 2021

This goto statement is important for improvement performance. Please try to run benchmark

@IncSW
Copy link
Contributor Author

IncSW commented May 3, 2021

I checked it in linux and windows, result same, tiny improvements with loop.

@IncSW
Copy link
Contributor Author

IncSW commented May 5, 2021

@goccy
https://godbolt.org/z/Kv4o49dT7 assembly is same.

@goccy
Copy link
Owner

goccy commented May 5, 2021

I'm planning to do the refactoring after this, so I don't need to create a PR at this stage.
But this fix is ​​good 👍 Thanks !

@codecov-commenter
Copy link

Codecov Report

Merging #212 (a52bba9) into master (5190536) will increase coverage by 0.02%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #212      +/-   ##
==========================================
+ Coverage   82.12%   82.15%   +0.02%     
==========================================
  Files          49       49              
  Lines       21765    21768       +3     
==========================================
+ Hits        17875    17883       +8     
+ Misses       3026     3021       -5     
  Partials      864      864              

@goccy goccy merged commit 46bdba4 into goccy:master May 6, 2021
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.

3 participants