Introduce CachingSession instead of Session #155

ZuzaOsa · Mar 10, 2025

This PR introduces caching session for optimizing the execution of prepared statements. It also adds support for the maxPrepared option of Client and changes the default value of the cache from 500 to 512.

Slightly improves performance and significantly improves amount of transferred data (number of packet measured with wireshark):
Before:

2.74 milion packets transferred

sudo perf stat node ./benchmark/logic/concurrent_insert.js scylladb-javascript-driver 1000000


 Performance counter stats for 'node ./benchmark/logic/concurrent_insert.js scylladb-javascript-driver 1000000':

        105,883.59 msec task-clock                       #    1.908 CPUs utilized             
         3,518,881      context-switches                 #   33.233 K/sec                     
           116,370      cpu-migrations                   #    1.099 K/sec                     
           191,451      page-faults                      #    1.808 K/sec                     
   426,824,192,831      cycles                           #    4.031 GHz                       
   129,342,909,399      stalled-cycles-frontend          #   30.30% frontend cycles idle      
   310,650,304,423      instructions                     #    0.73  insn per cycle            
                                                  #    0.42  stalled cycles per insn   
    59,643,874,497      branches                         #  563.297 M/sec                     
     2,479,172,913      branch-misses                    #    4.16% of all branches           

      55.494683305 seconds time elapsed

      72.364812000 seconds user
      35.192590000 seconds sys

After:

1.47 milion packets transferred

sudo perf stat node ./benchmark/logic/concurrent_insert.js scylladb-javascript-driver 1000000

 Performance counter stats for 'node ./benchmark/logic/concurrent_insert.js scylladb-javascript-driver 1000000':

         89,110.66 msec task-clock                       #    1.644 CPUs utilized             
         3,253,277      context-switches                 #   36.508 K/sec                     
            79,698      cpu-migrations                   #  894.371 /sec                      
           190,646      page-faults                      #    2.139 K/sec                     
   357,511,281,194      cycles                           #    4.012 GHz                       
    95,049,526,873      stalled-cycles-frontend          #   26.59% frontend cycles idle      
   272,851,008,791      instructions                     #    0.76  insn per cycle            
                                                  #    0.35  stalled cycles per insn   
    52,113,507,640      branches                         #  584.818 M/sec                     
     1,816,498,178      branch-misses                    #    3.49% of all branches           

      54.199610935 seconds time elapsed

      62.085423000 seconds user
      28.553035000 seconds sys

src/session.rs

Copilot

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

src/session.rs:27

[nitpick] The new field is named 'cache_size' in SessionOptions while the corresponding JavaScript option is called 'maxPrepared'. For consistency, consider aligning the naming between the Rust and JavaScript sides.

pub cache_size: Option<u32>,

lib/client-options.js

src/session.rs

Copilot

Pull Request Overview

This PR introduces a caching session to optimize the execution of prepared statements, adds a new cache_size option for sessions, and updates the client options to support a new maxPrepared parameter with a default cache size of 512.

Replaces Session with CachingSession in the session wrapper and adjusts method calls accordingly.
Introduces a new cache_size field in SessionOptions with a default of 512.
Updates client-options.js to change the default maxPrepared value and maps it to the Rust option cacheSize.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
src/session.rs	Replaces Session with CachingSession and adds cache_size handling.
lib/client-options.js	Updates maxPrepared default and maps it to cacheSize in Rust options.

Comments suppressed due to low confidence (2)

lib/client-options.js:670

[nitpick] The naming of the cache option is inconsistent between JS (maxPrepared) and Rust (cache_size); consider unifying the naming conventions for clarity.

rustOptions.cacheSize = options.maxPrepared;

lib/client-options.js:332

The default value for maxPrepared is now set to null, which may conflict with the PR's intent to change the default cache value from 500 to 512; please verify that the fallback to DEFAULT_CACHE_SIZE in the Rust code is sufficiently handling this scenario or consider aligning the default value.

maxPrepared: null,

adespawn

One small thing. Otherwise LGTM

lib/client-options.js

adespawn · Apr 16, 2025

I will try to run benchmarks for this PR in a moment

ZuzaOsa force-pushed the caching-session branch from 08f9da7 to 4f08298 Compare March 10, 2025 21:15

ZuzaOsa force-pushed the second-iteration branch from d0d471c to 7a1a8c8 Compare March 20, 2025 09:34

adespawn force-pushed the caching-session branch from 4f08298 to 9bf7852 Compare March 21, 2025 09:58

wprzytula added this to the Iteration 2 milestone Mar 21, 2025

ZuzaOsa force-pushed the caching-session branch from 9bf7852 to ed21797 Compare March 25, 2025 07:54

ZuzaOsa marked this pull request as ready for review March 25, 2025 07:54

ZuzaOsa requested review from adespawn and wprzytula March 25, 2025 07:55

adespawn reviewed Mar 25, 2025

View reviewed changes

src/session.rs Outdated Show resolved Hide resolved

wprzytula requested changes Mar 25, 2025

View reviewed changes

src/session.rs Outdated Show resolved Hide resolved

src/session.rs Outdated Show resolved Hide resolved

src/session.rs Outdated Show resolved Hide resolved

wprzytula assigned ZuzaOsa Mar 25, 2025

ZuzaOsa mentioned this pull request Mar 25, 2025

Add documentation of statement execution #169

Open

wprzytula requested changes Mar 26, 2025

View reviewed changes

src/session.rs Outdated Show resolved Hide resolved

ZuzaOsa force-pushed the caching-session branch 2 times, most recently from f5de2c4 to b727505 Compare April 8, 2025 06:32

wprzytula requested changes Apr 8, 2025

View reviewed changes

src/session.rs Outdated Show resolved Hide resolved

src/session.rs Outdated Show resolved Hide resolved

src/session.rs Outdated Show resolved Hide resolved

src/session.rs Outdated Show resolved Hide resolved

adespawn mentioned this pull request Apr 11, 2025

Caching of prepared queries #166

Merged

ZuzaOsa force-pushed the caching-session branch from b727505 to b8bf151 Compare April 13, 2025 21:47

wprzytula self-requested a review April 15, 2025 10:25

PiotrJunior requested a review from Copilot April 15, 2025 10:25

Copilot AI reviewed Apr 15, 2025

View reviewed changes

lib/client-options.js Outdated Show resolved Hide resolved

wprzytula approved these changes Apr 15, 2025

View reviewed changes

adespawn force-pushed the second-iteration branch from 5712c85 to 6f4c9e6 Compare April 15, 2025 11:25

ZuzaOsa force-pushed the caching-session branch from b8bf151 to 9fb315b Compare April 16, 2025 09:50

ZuzaOsa requested a review from adespawn April 16, 2025 09:50

ZuzaOsa force-pushed the caching-session branch from 9fb315b to 4e72e2e Compare April 16, 2025 09:56

adespawn requested changes Apr 16, 2025

View reviewed changes

lib/client-options.js Show resolved Hide resolved

lib/client-options.js Outdated Show resolved Hide resolved

src/session.rs Show resolved Hide resolved

src/session.rs Outdated Show resolved Hide resolved

ZuzaOsa force-pushed the caching-session branch from 4e72e2e to a11aeab Compare April 16, 2025 13:06

ZuzaOsa requested review from adespawn and Copilot April 16, 2025 13:07

Copilot AI reviewed Apr 16, 2025

View reviewed changes

adespawn approved these changes Apr 16, 2025

View reviewed changes

lib/client-options.js Outdated Show resolved Hide resolved

Replace Session with CachingSession

2bd4fbb

ZuzaOsa force-pushed the caching-session branch from a11aeab to 2bd4fbb Compare April 16, 2025 13:36

ZuzaOsa merged commit 987a699 into second-iteration Apr 17, 2025
8 checks passed

adespawn mentioned this pull request Apr 18, 2025

Execution options ~caching~ #164

Merged

adespawn deleted the caching-session branch April 21, 2025 10:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Introduce CachingSession instead of Session #155

Introduce CachingSession instead of Session #155

ZuzaOsa commented Mar 10, 2025 •

edited by adespawn

Loading

Copilot AI left a comment

Copilot AI left a comment

adespawn left a comment

adespawn commented Apr 16, 2025

Search code, repositories, users, issues, pull requests...

Introduce CachingSession instead of Session #155

Introduce CachingSession instead of Session #155

Conversation

ZuzaOsa commented Mar 10, 2025 • edited by adespawn Loading

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

adespawn left a comment

Choose a reason for hiding this comment

adespawn commented Apr 16, 2025

ZuzaOsa commented Mar 10, 2025 •

edited by adespawn

Loading