Skip to content

perf: optimize walk method by 10-34% #179

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 1 commit into from
Sep 2, 2022

Conversation

jacobroschen
Copy link
Collaborator

The walk method now only takes the parameters it actually needs, which
gives a performance bump to due to less copying & passing of parameters
to the walk method.

Running json-querying-performance-testing, this change produced the
following speed improvements compared to v7.1.0.

smallCityLots
┌───────────────┬───────────┬────────────┬─────────────┐
│    (index)    │  shallow  │    deep    │ conditional │
├───────────────┼───────────┼────────────┼─────────────┤
│ jsonpath-plus │   0.0641  │   0.3249   │   0.1199    │
├───────────────┼───────────┼────────────┼─────────────┤
│  jsonpath-pr  │   0.0439  │   0.2930   │   0.0850    │
├───────────────┼───────────┼────────────┼─────────────┤
│  Time Delta   │  -0.0202  │  -0.0319   │  -0.0349    │
├───────────────┼───────────┼────────────┼─────────────┤
│   % Change    │ -31.5133% │  -9.8184%  │ -29.1076%   │
└───────────────┴───────────┴────────────┴─────────────┘
mediumCityLots
┌───────────────┬───────────┬────────────┬─────────────┐
│    (index)    │  shallow  │    deep    │ conditional │
├───────────────┼───────────┼────────────┼─────────────┤
│ jsonpath-plus │   0.092   │   0.6739   │   0.2409    │
├───────────────┼───────────┼────────────┼─────────────┤
│  jsonpath-pr  │   0.0659  │   0.5443   │   0.1672    │
├───────────────┼───────────┼────────────┼─────────────┤
│  Time Delta   │  -0.0261  │  -0.1296   │  -0.0737    │
├───────────────┼───────────┼────────────┼─────────────┤
│   % Change    │ -28.3696% │ -19.2313%  │ -30.5936%   │
└───────────────┴───────────┴────────────┴─────────────┘
largeCityLots
┌───────────────┬───────────┬────────────┬─────────────┐
│    (index)    │  shallow  │    deep    │ conditional │
├───────────────┼───────────┼────────────┼─────────────┤
│ jsonpath-plus │   0.1910  │   1.8946   │   0.4900    │
├───────────────┼───────────┼────────────┼─────────────┤
│  jsonpath-pr  │   0.1347  │   1.6538   │   0.3230    │
├───────────────┼───────────┼────────────┼─────────────┤
│  Time Delta   │   0.0563  │  -0.2408   │  -0.1670    │
├───────────────┼───────────┼────────────┼─────────────┤
│   % Change    │ -29.4764% │ -12.7098%  │ -34.0816%   │
└───────────────┴───────────┴────────────┴─────────────┘

PR description

Checklist

  • - Added tests
  • - Ran npm test, ensuring linting passes
  • - Adjust README documentation if relevant

The walk method now only takes the parameters it actually needs, which
gives a performance bump to due to less copying & passing of parameters
to the walk method.

Running json-querying-performance-testing, this change produced the
following speed improvements compared to v7.1.0.

```
smallCityLots
┌───────────────┬───────────┬────────────┬─────────────┐
│    (index)    │  shallow  │    deep    │ conditional │
├───────────────┼───────────┼────────────┼─────────────┤
│ jsonpath-plus │   0.0641  │   0.3249   │   0.1199    │
├───────────────┼───────────┼────────────┼─────────────┤
│  jsonpath-pr  │   0.0439  │   0.2930   │   0.0850    │
├───────────────┼───────────┼────────────┼─────────────┤
│  Time Delta   │  -0.0202  │  -0.0319   │  -0.0349    │
├───────────────┼───────────┼────────────┼─────────────┤
│   % Change    │ -31.5133% │  -9.8184%  │ -29.1076%   │
└───────────────┴───────────┴────────────┴─────────────┘
mediumCityLots
┌───────────────┬───────────┬────────────┬─────────────┐
│    (index)    │  shallow  │    deep    │ conditional │
├───────────────┼───────────┼────────────┼─────────────┤
│ jsonpath-plus │   0.092   │   0.6739   │   0.2409    │
├───────────────┼───────────┼────────────┼─────────────┤
│  jsonpath-pr  │   0.0659  │   0.5443   │   0.1672    │
├───────────────┼───────────┼────────────┼─────────────┤
│  Time Delta   │  -0.0261  │  -0.1296   │  -0.0737    │
├───────────────┼───────────┼────────────┼─────────────┤
│   % Change    │ -28.3696% │ -19.2313%  │ -30.5936%   │
└───────────────┴───────────┴────────────┴─────────────┘
largeCityLots
┌───────────────┬───────────┬────────────┬─────────────┐
│    (index)    │  shallow  │    deep    │ conditional │
├───────────────┼───────────┼────────────┼─────────────┤
│ jsonpath-plus │   0.1910  │   1.8946   │   0.4900    │
├───────────────┼───────────┼────────────┼─────────────┤
│  jsonpath-pr  │   0.1347  │   1.6538   │   0.3230    │
├───────────────┼───────────┼────────────┼─────────────┤
│  Time Delta   │   0.0563  │  -0.2408   │  -0.1670    │
├───────────────┼───────────┼────────────┼─────────────┤
│   % Change    │ -29.4764% │ -12.7098%  │ -34.0816%   │
└───────────────┴───────────┴────────────┴─────────────┘
```
@jacobroschen jacobroschen requested a review from brettz9 September 2, 2022 19:45
@jacobroschen
Copy link
Collaborator Author

@brettz9 Once this is approved & merged I would appreciate if you could cut a new release.

@jacobroschen jacobroschen changed the title perf: optimize walk method by 10%-34% perf: optimize walk method by 10-34% Sep 2, 2022
Copy link
Collaborator

@brettz9 brettz9 left a comment

Choose a reason for hiding this comment

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

LGTM!

@brettz9
Copy link
Collaborator

brettz9 commented Sep 2, 2022

Go ahead and merge if you have no further last minute changes. I can then look to release.

@jacobroschen jacobroschen merged commit c32644e into JSONPath-Plus:main Sep 2, 2022
@jacobroschen jacobroschen deleted the jacob/perf/walk branch September 5, 2022 23:12
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