Skip to content

test(NODE-5692): Add new benchmarks using bson-bench #636

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

Merged
merged 40 commits into from
Nov 20, 2023
Merged

Conversation

W-A-James
Copy link
Contributor

@W-A-James W-A-James commented Nov 2, 2023

Description

What is changing?

Important

Note to reviewers The diff-size of this PR is large mostly due to the addition of the generated test files. To ensure that these files have been generated properly, review test/bench/etc/generate_documents.ts and look at the smallest file possible that relates to the case being reviewed.

e.g. if reviewing how nested test documents were generated, look at nested_4.json as the larger documents are build in the same fashion, but will be difficult to load into an editor

New benchmarks
  • Adds new granular benchmarks to test/bench/granular
    • Note that these new benchmarks directly implement the AC for deserialization and serialization tests laid out in the ticket
  • Adds bson spec benchmarks to test/bench/spec
    • Note that these tests perform the migration of bson benchmark testing from the driver repository to the js-bson repository and will require the removal of the bsonBench tests from the driver repository in a follow-up PR
  • Adds test documents to test/bench/documents
    • These tests have all been generated using the new file added to test/bench/etc/generate_documents.ts and understanding these documents is better done by looking at the logic in this file
  • Adds new utility scripts to test/bench/etc
    • generate_documents.ts - Regenerates documents in test/bench/documents
    • run_granular_benchmarks.js - runs all compiled benchmarks in test/bench/lib/granular and consolidates results into resultsCollected.json in perf.send format
    • convertToCSV.js - Reads in a data in perf.send format from stdin and writes it to stdout converted to a csv more easily ingestible by google sheets, or other data-analysis tools
CI changes
  • Adds new CI task for granular benchmarks
    • Changed check:bench script in package.json to check:granular-bench
      • this script builds the benchmark typescript files in test/bench/granular and runs run_granular_benchmarks.js
    • Added run granular benchmarks function to config.yml
    • Added .evergreen/run-granular-benchmarks.sh
    • Added run-granular-benchmarks-node-18 task in place of run-benchmarks-node-18 task
    • Runs run-granular-benchmarks-node-18 task on perf distribution
  • Adds new CI task for spec benchmarks
    • Added check:spec-bench script in package.json
    • Added .evergreen/run-spec-benchmarks.sh
    • Added run spec benchmarks function to config.yml
    • Added run-spec-benchmarks-node-18 task
    • Runs run-granular-benchmarks-node-18 task on perf distribution
Removals
  • Removes old benchmarking infrastructure from etc/benchmarks
  • Removes old benchmarking infrastructure from test/bench/src
Is there new documentation needed for these changes?

No

What is the motivation for this change?

NODE-5692

Release Highlight

Fill in title or leave empty for no highlight

Double check the following

  • Ran npm run check:lint script
  • Self-review completed using the steps outlined here
  • PR title follows the correct format: type(NODE-xxxx)[!]: description
    • Example: feat(NODE-1234)!: rewriting everything in coffeescript
  • Changes are covered by tests
  • New TODOs have a related JIRA ticket

Sorry, something went wrong.

@W-A-James W-A-James marked this pull request as ready for review November 10, 2023 19:07
@nbbeeken nbbeeken self-assigned this Nov 15, 2023
@nbbeeken nbbeeken self-requested a review November 15, 2023 20:02
@nbbeeken nbbeeken added the Primary Review In Review with primary reviewer, not yet ready for team's eyes label Nov 15, 2023
@W-A-James W-A-James requested a review from nbbeeken November 16, 2023 19:43
Copy link
Contributor

@nbbeeken nbbeeken left a comment

Choose a reason for hiding this comment

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

Small follow up Qs, LGTM

@nbbeeken nbbeeken added Team Review Needs review from team and removed Primary Review In Review with primary reviewer, not yet ready for team's eyes labels Nov 20, 2023
@nbbeeken nbbeeken merged commit a382485 into main Nov 20, 2023
@nbbeeken nbbeeken deleted the NODE-5692 branch November 20, 2023 19:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Team Review Needs review from team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants