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.

Sign up for GitHub

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

Jump to bottom

Fix json.arrappend performance #324

Merged
merged 9 commits into from Apr 29, 2021
Merged

Fix json.arrappend performance #324

merged 9 commits into from Apr 29, 2021

Conversation

gkorland
Copy link
Contributor

@gkorland gkorland commented Apr 24, 2021 •
edited

fix #295

@gkorland gkorland added the bug label Apr 25, 2021
@gkorland gkorland changed the title Arr append Fix json.arrappend performance Apr 25, 2021
@filipecosta90
Copy link
Contributor

@gkorland even tough an impressive improvement, if we run the arrappend test for a long time v1.0.7 still outperforms the current branch. Noneteless I would urge us to merge this PR given we bump from 50 ops/sec to 70K:

v1.0.7 branch:

memtier_benchmark --command '"JSON.ARRAPPEND" "path:sonde:foo" ".properties.coordinateProperties" "{\"type\": \"Feature\", \"geometry\": {\"type\": \"Point\", \"coordinates\": [26.2153, 60.34085, 417.0]}, \"properties\": {\"temp\": 3.6, \"serial\": \"S0730589\", \"humidity\": 64.6, \"vel_h\": 16.0, \"uploader_callsign\": \"Lahti 24h raspberry\", \"time\": 1617220020, \"uploader\": {\"callsign\": \"Lahti 24h raspberry\"}}}"' --test-time 120 --json-out-file 5k.arrappend-results.json
Json file 5k.arrappend-results.json created...
Writing results to stdout
[RUN #1] Preparing benchmark client...
[RUN #1] Launching threads now...
[RUN #1 100%, 120 secs]  0 threads:    12446983 ops,  102956 (avg:  103722) ops/sec, 38.69MB/sec (avg: 38.89MB/sec),  1.94 (avg:  1.93) msec latency

4         Threads
50        Connections per thread
120       Seconds


ALL STATS
============================================================================================================
Type                   Ops/sec    Avg. Latency     p50 Latency     p99 Latency   p99.9 Latency       KB/sec 
------------------------------------------------------------------------------------------------------------
"json.arrappend"s    103723.15         1.92833         1.91900         3.21500         3.31100     39818.68 
Totals               103723.15         1.92833         1.91900         3.21500         3.31100     39818.68

arr_append branch:

memtier_benchmark --command '"JSON.ARRAPPEND" "path:sonde:foo" ".properties.coordinateProperties" "{\"type\": \"Feature\", \"geometry\": {\"type\": \"Point\", \"coordinates\": [26.2153, 60.34085, 417.0]}, \"properties\": {\"temp\": 3.6, \"serial\": \"S0730589\", \"humidity\": 64.6, \"vel_h\": 16.0, \"uploader_callsign\": \"Lahti 24h raspberry\", \"time\": 1617220020, \"uploader\": {\"callsign\": \"Lahti 24h raspberry\"}}}"' --test-time 120 --json-out-file 5k.arrappend-results.json
Json file 5k.arrappend-results.json created...
Writing results to stdout
[RUN #1] Preparing benchmark client...
[RUN #1] Launching threads now...
[RUN #1 100%, 120 secs]  0 threads:     9577973 ops,   79475 (avg:   79814) ops/sec, 29.79MB/sec (avg: 29.91MB/sec),  2.51 (avg:  2.51) msec latency

4         Threads
50        Connections per thread
120       Seconds


ALL STATS
============================================================================================================
Type                   Ops/sec    Avg. Latency     p50 Latency     p99 Latency   p99.9 Latency       KB/sec 
------------------------------------------------------------------------------------------------------------
"json.arrappend"s     79814.49         2.50583         2.47900         4.25500         4.38300     30628.58 
Totals                79814.49         2.50583         2.47900         4.25500         4.38300     30628.58

filipecosta90
filipecosta90 previously approved these changes Apr 26, 2021
View reviewed changes
@filipecosta90 filipecosta90 mentioned this pull request Apr 26, 2021
Closed
@gkorland gkorland marked this pull request as ready for review April 27, 2021 12:55
@gkorland gkorland requested a review from oshadmi April 28, 2021 15:27
@gkorland gkorland merged commit 00e9f9d into master Apr 29, 2021
@gkorland gkorland deleted the arr_append branch April 29, 2021 14:56
oshadmi added a commit that referenced this pull request May 5, 2021
@rafie @gkorland
@oshadmi @TomerHekmati @gavrie
[feature-search-json] Sync with master (2) (#344)
93e471f 
* Update to Redis 6.2.2 (#332)

* Fix json.arrappend performance (#324)

* improve arr_pop, arr_insert & arr_trim performance 
* refactor do_json_* functions

* normalize index for backward (#327)

* normalize index for backward
* adpat for full backward with 1.0.x

* fix #318 (#333)

* CircleCI: macOS and benchmark fixes (#337)

* Update config.yml (#338)

* Add JSON.CLEAR command (#306)

* Restore optional path (default is $) for JSON.DEL

* Add JSON.CLEAR

* Handle JSON.CLEAR with $ path

* Add JSON.CLEAR - Fix failure in test:testSetInvalidPathShouldFail

* Idiomatic fixes per Gavrie review

Co-authored-by: Guy Korland <gkorland@gmail.com>

* Update RS_VERSIONS

last promoted builds

* Document flags for running tests

* fix arr_pop backward to 1.0 (#345)

* JSON.INSERTARR backward compatability for 1.0 (#343)

* Fix JSON.INSERTARR backward compatibility with 1.0.x
* Update test.py

* Fix build error
(use of undeclared crate or module `rawmod`)

* Fix format

Co-authored-by: Guy Korland <gkorland@gmail.com>
Co-authored-by: Omer Shadmi <76992134+oshadmi@users.noreply.github.com>
Co-authored-by: tomerhekredis <72793005+tomerhekredis@users.noreply.github.com>
Co-authored-by: Gavrie Philipson <gavrie@philipson.co.il>
Co-authored-by: oshadmi <omer.shadmi@redislabs.com>
@filipecosta90 filipecosta90 mentioned this pull request May 11, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gavrie gavrie gavrie approved these changes

@filipecosta90 filipecosta90 filipecosta90 approved these changes

@oshadmi oshadmi Awaiting requested review from oshadmi

Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

JSON.ARRAPPEND: cpu bound
3 participants
@gkorland @filipecosta90 @gavrie