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.

Sign up for GitHub

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

Jump to bottom

Preload method with benchmarks #14

Merged
merged 2 commits into from
Jul 6, 2021
Merged

Preload method with benchmarks #14

merged 2 commits into from
Jul 6, 2021

Conversation

g4s8
Copy link
Contributor

@g4s8 g4s8 commented May 17, 2021

Fixes #13 - added preload method to initialize database without
performing any license check. Added two benchmarks to compare
the difference between calls with and without preload.

@g4s8
Preload method with benchmarks
b1270b3 
Fixes #13 - added preload method to initialize database without
performing any license check. Added two benchmarks to compare
the difference between calls with and without preload.
@g4s8
Copy link
Contributor Author

g4s8 commented May 17, 2021 •
edited

Benchmark results on my laptop:

$ go test -bench="BenchmarkDetect$" ./licensedb -run=BenchmarkDetect
goos: linux
goarch: amd64
pkg: github.com/go-enry/go-license-detector/v4/licensedb
cpu: Intel(R) Core(TM) i7-8750H CPU @ 2.20GHz
BenchmarkDetect-12    	      1	2183395428 ns/op
PASS
ok  	github.com/go-enry/go-license-detector/v4/licensedb	2.344s

$ go test -bench="BenchmarkDetectWithPreload$" ./licensedb -run=BenchmarkDetectWithPreload
goos: linux
goarch: amd64
pkg: github.com/go-enry/go-license-detector/v4/licensedb
cpu: Intel(R) Core(TM) i7-8750H CPU @ 2.20GHz
BenchmarkDetectWithPreload-12    	     76	 15373506 ns/op
PASS
ok  	github.com/go-enry/go-license-detector/v4/licensedb	3.444s

@g4s8
Copy link
Contributor Author

g4s8 commented May 17, 2021

@bzz could you please check this PR?

@g4s8
Copy link
Contributor Author

g4s8 commented Jul 6, 2021

@bzz hi, any chance to review this PR?

bzz
bzz approved these changes Jul 6, 2021
View reviewed changes
Copy link
Member

@bzz bzz left a comment •
edited

Choose a reason for hiding this comment

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

LGTM, sans a minor API doc clarification.

Windows test failures have nothing to do with the change.

Thank you for adding this!

licensedb/licensedb.go Outdated
@@ -53,3 +53,8 @@ func Detect(fs filer.Filer) (map[string]api.Match, error) {
}
return licenses, nil
}

// Preload database with licenses
Copy link
Member

Choose a reason for hiding this comment

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

As it extends the API surface, IMO it would be nice to add a few words here to clarify the contract - that it is an optimization and the client does need to call this first, as the rest of the APIs are doing that lazily anyway, or some words to that effect.

WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, will fix that

@g4s8
Copy link
Contributor Author

g4s8 commented Jul 6, 2021

LGTM, sans a minor API doc clarification.

Windows test failures have nothing to do with the change.

Thank you for adding this!

@bzz thanks, fixed in the last commit 6403a23

@bzz bzz merged commit 4f2ca6a into go-enry:master Jul 6, 2021
@g4s8 g4s8 deleted the 13-preload branch July 7, 2021 11:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bzz bzz bzz approved these changes

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Long initialization on first detect
2 participants
@g4s8 @bzz