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

fix: improve performance of loadProtoJSON #1196

Merged
merged 3 commits into from
Mar 4, 2022
Merged

Conversation

summer-ji-eng
Copy link
Contributor

@summer-ji-eng summer-ji-eng commented Mar 1, 2022

To address issue #1195

Problem:
Significant time cost to create hash for proto JSON for a larget protos set. Ads as example

const hash = objectHash(json).toString();

To improve, we have two options (thanks @avermeil's suggestions):

  1. Replace json object to json stirng.
const hash = objectHash(JSON.stringify(json)).toString();
  1. (Adopt, best performance)The third party library object-hash is build on top of crypto module. Use crypto directly.
const hash = crypto.createHash('md5').update(JSON.stringify(json)).digest('hex');

The time cost comparision using Language API:

option time cost
Origin 23.007ms
Option 1 1.049ms
Option 2 0.535ms

@summer-ji-eng summer-ji-eng requested a review from a team as a code owner March 1, 2022 18:02
src/fallback.ts Outdated Show resolved Hide resolved
src/grpc.ts Outdated Show resolved Hide resolved
src/grpc.ts Outdated Show resolved Hide resolved
@@ -357,7 +357,7 @@ describe('v1beta1.EchoClient', () => {
);
});

it.only('invokes echo with closed client', async () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Accidentally checked in before. Should remove only

@summer-ji-eng summer-ji-eng merged commit df8eaf9 into main Mar 4, 2022
@summer-ji-eng summer-ji-eng deleted the improve_load_proto branch March 4, 2022 06:40
gcf-merge-on-green bot pushed a commit that referenced this pull request Mar 8, 2022
🤖 I have created a release *beep* *boop*
---


### [2.30.1](v2.30.0...v2.30.1) (2022-03-08)


### Bug Fixes

* do not depend on index.ts from fallback code ([#1201](#1201)) ([5c7ca41](5c7ca41))
* improve performance of loadProtoJSON ([#1196](#1196)) ([df8eaf9](df8eaf9))

---
This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants