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

Improved strictness and speed of regexp matching #312 #339

Merged
merged 1 commit into from Nov 10, 2017
Merged

Improved strictness and speed of regexp matching #312 #339

merged 1 commit into from Nov 10, 2017

Conversation

yulvil
Copy link
Contributor

@yulvil yulvil commented Nov 10, 2017

For #312

The regexps are now stricter, i.e. need to match the whole line.

There is a minimal speed improvement by skipping the NodeName.

$ go test -run=NONE -bench=. -benchtime=10s > old
[change code]
$ go test -run=NONE -bench=. -benchtime=10s > new

$ go install golang.org/x/tools/cmd/benchcmp
$ benchcmp old new
benchmark          old ns/op     new ns/op     delta
BenchmarkParse     27993406      25026323      -10.60%

@elliotchance Is it possible to leave the issue open after this merge? I would like to make more changes.


This change is Reviewable

@codecov
Copy link

codecov bot commented Nov 10, 2017

Codecov Report

Merging #339 into master will increase coverage by 0.02%.
The diff coverage is 93.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #339      +/-   ##
==========================================
+ Coverage    81.6%   81.62%   +0.02%     
==========================================
  Files         142      142              
  Lines        6011     6013       +2     
==========================================
+ Hits         4905     4908       +3     
+ Misses        886      885       -1     
  Partials      220      220
Impacted Files Coverage Δ
ast/function_decl.go 93.93% <100%> (-0.51%) ⬇️
ast/record_decl.go 94.28% <100%> (+0.16%) ⬆️
ast/var_decl.go 94.44% <100%> (+0.15%) ⬆️
ast/typedef_decl.go 93.93% <100%> (+0.18%) ⬆️
ast/ast.go 98% <83.33%> (+0.52%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b60daca...8a508d9. Read the comment docs.

@elliotchance
Copy link
Owner

Reviewed 9 of 9 files at r1.
Review status: all files reviewed at latest revision, all discussions resolved.


Comments from Reviewable

@elliotchance elliotchance merged commit 1618013 into elliotchance:master Nov 10, 2017
@yulvil yulvil deleted the 312regexp branch November 10, 2017 18:58
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