Skip to content
This repository has been archived by the owner on Feb 18, 2024. It is now read-only.

Improved performance in lexical write (~5%) #1067

Merged
merged 2 commits into from
Jun 10, 2022

Conversation

ritchie46
Copy link
Collaborator

It might be interesting to explore a slightly adapted version of BufStreamingIterator.

If I am correct it currently writes to an internal buffer, and then copies that buffer to the W: Write buffer. If we would accept that final buffer in the next() method as argument, we could directly write to that.

Will explore that idea later.

@codecov
Copy link

codecov bot commented Jun 10, 2022

Codecov Report

Merging #1067 (8b84f3d) into main (6919cc3) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main    #1067   +/-   ##
=======================================
  Coverage   81.30%   81.31%           
=======================================
  Files         363      363           
  Lines       34689    34700   +11     
=======================================
+ Hits        28205    28217   +12     
+ Misses       6484     6483    -1     
Impacted Files Coverage Δ
src/util/lexical.rs 76.47% <100.00%> (+11.25%) ⬆️
src/io/ipc/read/stream_async.rs 74.38% <0.00%> (+0.82%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6919cc3...8b84f3d. Read the comment docs.

@jorgecarleitao jorgecarleitao added the enhancement An improvement to an existing feature label Jun 10, 2022
@jorgecarleitao
Copy link
Owner

Looks reasonable! Do you have a bench on performance delta? Curious how it impacts.

@ritchie46
Copy link
Collaborator Author

Yup

bench_1                 time:   [1.0596 ms 1.0599 ms 1.0601 ms]                     
                        change: [-5.8026% -5.3279% -4.9843%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 6 outliers among 100 measurements (6.00%)
  4 (4.00%) low mild
  2 (2.00%) high mild

pub fn compute(vals: &[i32], buf: &mut Vec<u8>)  {
    for v in vals {
        memcheck::lexical_to_bytes_mut_unchecked(*v, buf)
    }
}

fn add_benchmark(c: &mut Criterion) {

    let mut rng = thread_rng();
    let values = (0..100000)
        .map(|_| rng.sample(Standard))
        .collect::<Vec<i32>>();

    let mut buf = vec![];

    c.bench_function("bench_1", |b| {
        b.iter(|| black_box(compute(&values, &mut buf)))
    });
}

@ritchie46 ritchie46 changed the title omit bound check in lexical write omit bound check in lexical write: ~5% Jun 10, 2022
@jorgecarleitao jorgecarleitao changed the title omit bound check in lexical write: ~5% Improved performance in lexical write (~5%) Jun 10, 2022
@jorgecarleitao jorgecarleitao merged commit d2f5935 into jorgecarleitao:main Jun 10, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement An improvement to an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants