Avoid including page loading time in project expression profiler#13137
Avoid including page loading time in project expression profiler#13137raunaqmorarka wants to merge 1 commit intotrinodb:masterfrom
Conversation
There was a problem hiding this comment.
where it will be accounted for?
There was a problem hiding this comment.
I'm not yet sure that this needs to be accounted for anywhere.
My thought was that if ExpressionProfiler is about detecting time consuming expression evaluation, then the addition of page loading time, which includes orc/parquet read from filesystem and decoding, is probably not right.
But I wasn't certain about it.
There was a problem hiding this comment.
I'm not yet sure that this needs to be accounted for anywhere.
i would bet that it needs to
My thought was that if ExpressionProfiler is about detecting time consuming expression evaluation
sounds so.
There was a problem hiding this comment.
I did some benchmarking found no change to TPC and small improvement in scan operator benchmarks.
The job of Expression Profiler appears to be to reduce projection batch sizes for expensive expressions so that they don't hog CPU for too long. Any change to projection batch size due to this heuristic does not impact the size of reads and the pages produced by orc/parquet (those are based on different criteria like locality of reads and memory consumption). So it seems that Expression Profiler should ignore time taken to load page from the page source.
raunaqmorarka
force-pushed
the
exp-pp
branch
from
a44c363 to
b5327e4
Compare
|
Moved to #14135 as a prep commit |
2 participants
Description
Related issues, pull requests, and links
Documentation
( ) No documentation is needed.
( ) Sufficient documentation is included in this PR.
( ) Documentation PR is available with #prnumber.
( ) Documentation issue #issuenumber is filed, and can be handled later.
Release notes
( ) No release notes entries required.
( ) Release notes entries required with the following suggested text: