theirix commented on PR #21383:
URL: https://github.com/apache/datafusion/pull/21383#issuecomment-4202330345

   > Looks like there is some (reproducable) slow down on some of the null 
cases:
   > 
   > ```
   > group                                                           main       
                            optimise-first_last
   > -----                                                           ----       
                            -------------------
   > first_value convert_to_state nulls=90%, filter=false            1.00     
55.7±0.35µs        ? ?/sec    1.00     55.8±0.32µs        ? ?/sec
   > first_value convert_to_state nulls=90%, filter=true             1.00    
794.5±2.42ns        ? ?/sec    1.00    795.1±3.59ns        ? ?/sec
   > last_value evaluate_accumulator_bench nulls=0%                  1.00    
139.1±6.85ns        ? ?/sec    1.17  162.8±101.41ns        ? ?/sec
   > last_value evaluate_accumulator_bench nulls=90%                 1.00    
138.7±6.61ns        ? ?/sec    1.08    149.2±9.36ns        ? ?/sec
   > ```
   > 
   > @theirix can you look into that?
   
   Just checked - evaluate_accumulator_bench is a very noisy test for a trivial 
`FirstValueAccumulator::evaluate` method, that runs a `ScalarValue` clone. The 
variance of this test is also close to the measurement, both around 100ns, 
which proves the unreliability of this measurement.
   
   I can drop this `evaluate_accumulator_bench` benchmark to avoid noise. The 
focus of benchmarking should be on heavier methods like `update_batch`, 
`merge_batch`


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to