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

[chore] - update benchmarks. #1641

Merged
merged 2 commits into from
Aug 23, 2023
Merged

[chore] - update benchmarks. #1641

merged 2 commits into from
Aug 23, 2023

Conversation

ahrav
Copy link
Collaborator

@ahrav ahrav commented Aug 18, 2023

Description:

Reset the timer before running the benchmark to exclude setup time.
Use a helper to create the benchmark data that is programmatically created and is not dependent on the data within the detectors package. This will ensure that all benchmarks will always have the same amount of data. Include an additional size for the benchmark.

Checklist:

  • Tests passing (make test-community)?
  • Lint passing (make lint this requires golangci-lint)?

@ahrav ahrav marked this pull request as ready for review August 23, 2023 15:23
@ahrav ahrav requested a review from a team as a code owner August 23, 2023 15:23
Copy link
Collaborator

@mcastorina mcastorina left a comment

Choose a reason for hiding this comment

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

reviewing this PR is lagging out GH lol

Comment on lines 111 to 112
b.ResetTimer()
b.ResetTimer()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Double reset here

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ah whoops thanks, yea i was probably testing with this one and it got added with the bulk find and replace.

@mcastorina
Copy link
Collaborator

FYI for anyone curious, I did not review this in the GitHub UI. Instead, I checked it out locally and manually reviewed the files that had more than one change:

» gd --compact-summary HEAD~1 | grep -v '1 +'
 pkg/detectors/aha/aha_test.go                      |  2 ++
 pkg/detectors/bulksms/bulksms_test.go              |  6 ++--
 pkg/detectors/demio/demio_test.go                  |  5 +--
 pkg/detectors/detectors.go                         | 40 +++++++++++++---------
 pkg/detectors/formsite/formsite_test.go            |  5 +--
 pkg/detectors/salesmate/salesmate_test.go          |  5 +--
 pkg/detectors/scalr/scalr_test.go                  |  6 ++--
 pkg/detectors/tokeet/tokeet_test.go                |  6 ++--
 pkg/detectors/websitepulse/websitepulse_test.go    |  5 +--
 pkg/detectors/zapierwebhook/zapierwebhook_test.go  |  3 +-
 785 files changed, 823 insertions(+), 35 deletions(-)

@ahrav ahrav merged commit a5fbc54 into main Aug 23, 2023
@ahrav ahrav deleted the chore-fix-detector-benchmarks branch August 23, 2023 21:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants