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

Refactor: get object value by key name improve performance #79

Merged
merged 1 commit into from
Mar 26, 2025

Conversation

b41sh
Copy link
Member

@b41sh b41sh commented Mar 25, 2025

  1. Refactor the implementation of get object value by the key name, avoid reading all the key jentries at one time, change to a lazy mode, only read when need, avoid the performance impact of reading a lot of keys.
  2. Check whether the length of the object's key is equal to the needed key name first, to reduce the call of equal function.
  3. Avoid using Deserializer to convert the type of data inside the function, because usually we already know the type, we can extract it directly to avoid extra judgement in the conversion process of Deserializer.

@Xuanwo
Copy link
Member

Xuanwo commented Mar 25, 2025

Hi, how about we add a bench first?

@b41sh
Copy link
Member Author

b41sh commented Mar 26, 2025

Hi, how about we add a bench first?

@Xuanwo Thanks for your advice. By running the get_path benchmark, we can see a performance improvement of about 50 percent, and the tests at Databend have achieved similar results.

the main branch

     Running benches/get_path.rs (target/release/deps/get_path-02e1adcdc0056503)
Gnuplot not found, using plotters backend
jsonb get canada->type  time:   [105.38 ns 107.12 ns 109.33 ns]
Found 13 outliers among 100 measurements (13.00%)
  3 (3.00%) high mild
  10 (10.00%) high severe

jsonb get citm_catalog->areaNames->205705994
                        time:   [159.96 ns 161.45 ns 163.17 ns]
Found 6 outliers among 100 measurements (6.00%)
  2 (2.00%) high mild
  4 (4.00%) high severe

jsonb get citm_catalog->topicNames->324846100
                        time:   [190.09 ns 193.88 ns 198.33 ns]
Found 18 outliers among 100 measurements (18.00%)
  9 (9.00%) high mild
  9 (9.00%) high severe

jsonb get twitter->search_metadata->max_id_str
                        time:   [154.26 ns 160.10 ns 166.94 ns]
Found 10 outliers among 100 measurements (10.00%)
  2 (2.00%) high mild
  8 (8.00%) high severe

this PR

     Running benches/get_path.rs (target/release/deps/get_path-02e1adcdc0056503)
Gnuplot not found, using plotters backend
jsonb get canada->type  time:   [58.256 ns 59.683 ns 61.254 ns]
                        change: [-46.353% -45.295% -44.208%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 16 outliers among 100 measurements (16.00%)
  3 (3.00%) high mild
  13 (13.00%) high severe

jsonb get citm_catalog->areaNames->205705994
                        time:   [78.413 ns 80.497 ns 83.411 ns]
                        change: [-51.182% -50.129% -48.901%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 11 outliers among 100 measurements (11.00%)
  5 (5.00%) high mild
  6 (6.00%) high severe

jsonb get citm_catalog->topicNames->324846100
                        time:   [82.818 ns 84.680 ns 87.161 ns]
                        change: [-57.390% -56.545% -55.668%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 17 outliers among 100 measurements (17.00%)
  4 (4.00%) high mild
  13 (13.00%) high severe

jsonb get twitter->search_metadata->max_id_str
                        time:   [70.169 ns 71.039 ns 72.024 ns]
                        change: [-55.906% -54.712% -53.614%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 14 outliers among 100 measurements (14.00%)
  5 (5.00%) high mild
  9 (9.00%) high severe

@b41sh b41sh requested review from sundy-li and Xuanwo March 26, 2025 09:47
@sundy-li sundy-li merged commit 1a53512 into databendlabs:main Mar 26, 2025
1 check passed
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.

3 participants