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

perf(NODE-5875): use builtin native crypto #18

Closed
wants to merge 9 commits into from

Conversation

nbbeeken
Copy link
Collaborator

@nbbeeken nbbeeken commented Jun 13, 2024

Description

What is changing?

Is there new documentation needed for these changes?

What is the motivation for this change?

Increase decryption performance

BenchmarkRunner is using libmongocryptVersion=1.10.0, warmupSecs=2, testInSecs=57
samples taken:  2
samples taken:  57
Decrypting 1500 fields median ops/sec : 415

Release Highlight

Improve performance by using bundled native crypto APIs

Previously the bindings would rely on the cryptoCallbacks option which was an object with javascript functions corresponding to the cipher algorithms used by libmongocrypt. Since supported versions of Node.js always bundle OpenSSL we can rely on the native APIs being available directly to libmongocrypt. This improves decryption performance ~x5.5 since data does not need to pass between native and javascript layers as frequently.

The bindings will check for the existence of OpenSSL APIs and will fallback to using the cryptoCallbacks if they are not present. This requires building with a version of libmongocrypt that does not link with OpenSSL.

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

@nbbeeken nbbeeken changed the title feat(NODE-5455): use builtin native crypto feat(NODE-6224): use builtin native crypto Jun 13, 2024
@nbbeeken nbbeeken force-pushed the make-crypto-callbacks-optional branch 2 times, most recently from 9bc08f1 to f05f37e Compare June 13, 2024 19:09
@nbbeeken nbbeeken mentioned this pull request Jun 13, 2024
5 tasks
@nbbeeken nbbeeken changed the title feat(NODE-6224): use builtin native crypto feat(NODE-5875): use builtin native crypto Jun 13, 2024
@nbbeeken nbbeeken changed the title feat(NODE-5875): use builtin native crypto perf(NODE-5875): use builtin native crypto Jun 18, 2024
@nbbeeken nbbeeken added the Blocked Blocked on other work label Jun 18, 2024
@nbbeeken
Copy link
Collaborator Author

After a tech chat discussion the changes here is not what we want. We will continue to compile with nocrypto but adjust to create callbacks in C++ instead of from JS.

Base automatically changed from NODE-5455-bench-fle to main June 18, 2024 18:58
@nbbeeken nbbeeken force-pushed the make-crypto-callbacks-optional branch from 1b587b8 to eb13cdb Compare June 18, 2024 19:55
@nbbeeken
Copy link
Collaborator Author

Preferred change: #25

@nbbeeken nbbeeken closed this Jun 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Blocked Blocked on other work
Projects
None yet
1 participant