Skip to content

src: improve KVStore API #31773

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closed
wants to merge 2 commits into from

Conversation

lundibundi
Copy link
Member

This adds const char* based APIs to KVStore to avoid multiple string
conversions (char -> Utf8 -> Local -> char etc.) when possible.

Unfortunately this only changes Get and Query methods as others have a hard dependency on Isolate in RealEnvStore as they need to call DateTimeConfigurationChangeNotification. Not sure how can we move forward with that.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

/cc @addaleax

@nodejs-github-bot nodejs-github-bot added the c++ Issues and PRs that require attention from people who are familiar with C++. label Feb 13, 2020
@nodejs-github-bot
Copy link
Collaborator

This adds `const char*` based APIs to KVStore to avoid multiple string
conversions (char -> Utf8 -> Local -> char etc.) when possible.
@lundibundi lundibundi force-pushed the kvstorage-improve-api branch from 8591a52 to d38ed6c Compare February 13, 2020 20:04
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@lundibundi lundibundi added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Feb 27, 2020
@nodejs-github-bot
Copy link
Collaborator

addaleax pushed a commit that referenced this pull request Mar 9, 2020
This adds `const char*` based APIs to KVStore to avoid multiple string
conversions (char -> Utf8 -> Local -> char etc.) when possible.

PR-URL: #31773
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
addaleax pushed a commit that referenced this pull request Mar 9, 2020
PR-URL: #31773
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@addaleax
Copy link
Member

addaleax commented Mar 9, 2020

Landed in e70705c...98f4429

@addaleax addaleax closed this Mar 9, 2020
MylesBorins pushed a commit that referenced this pull request Mar 9, 2020
This adds `const char*` based APIs to KVStore to avoid multiple string
conversions (char -> Utf8 -> Local -> char etc.) when possible.

PR-URL: #31773
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request Mar 9, 2020
PR-URL: #31773
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Mar 10, 2020
targos pushed a commit that referenced this pull request Apr 20, 2020
This adds `const char*` based APIs to KVStore to avoid multiple string
conversions (char -> Utf8 -> Local -> char etc.) when possible.

PR-URL: #31773
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
targos pushed a commit that referenced this pull request Apr 20, 2020
PR-URL: #31773
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. c++ Issues and PRs that require attention from people who are familiar with C++.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants