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

sql: WIP on changing SupportsVectorized check #55948

Closed
wants to merge 2 commits into from

Conversation

yuzefovich
Copy link
Member

To be filled.

@yuzefovich yuzefovich added the do-not-merge bors won't merge a PR with this label. label Oct 25, 2020
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@yuzefovich
Copy link
Member Author

yuzefovich commented Oct 25, 2020

This prototype shows the performance difference of -4.1%:

  • vectorize_row_count_threshold=1000:
Highest sequence written: 882813. Can be passed as --write-seq=R882813 to the next run.

_elapsed___errors_____ops(total)___ops/sec(cum)__avg(ms)__p50(ms)__p95(ms)__p99(ms)_pMax(ms)__total
 1800.0s        0       15336790         8520.4      0.8      0.9      1.4      2.1    192.9  read

_elapsed___errors_____ops(total)___ops/sec(cum)__avg(ms)__p50(ms)__p95(ms)__p99(ms)_pMax(ms)__total
 1800.0s        0         805417          447.5      1.9      1.8      2.9      4.2    192.9  write

_elapsed___errors_____ops(total)___ops/sec(cum)__avg(ms)__p50(ms)__p95(ms)__p99(ms)_pMax(ms)__result
 1800.0s        0       16142207         8967.9      0.9      0.9      1.8      2.5    192.9  
  • vectorize_row_count_threshold=0:
Highest sequence written: 849815. Can be passed as --write-seq=R849815 to the next run.

_elapsed___errors_____ops(total)___ops/sec(cum)__avg(ms)__p50(ms)__p95(ms)__p99(ms)_pMax(ms)__total
 1800.0s        0       14710242         8172.4      0.9      0.9      1.5      2.4     35.7  read

_elapsed___errors_____ops(total)___ops/sec(cum)__avg(ms)__p50(ms)__p95(ms)__p99(ms)_pMax(ms)__total
 1800.0s        0         775295          430.7      1.9      1.9      2.9      4.5     88.1  write

_elapsed___errors_____ops(total)___ops/sec(cum)__avg(ms)__p50(ms)__p95(ms)__p99(ms)_pMax(ms)__result
 1800.0s        0       15485537         8603.1      0.9      0.9      1.8      2.6     88.1  

I added some pooling of objects in the vectorized path, and now we're down to -3.35%:

  • vectorize_row_count_threshold=0 with pooling commit:
Highest sequence written: 856470. Can be passed as --write-seq=R856470 to the next run.

_elapsed___errors_____ops(total)___ops/sec(cum)__avg(ms)__p50(ms)__p95(ms)__p99(ms)_pMax(ms)__total
 1800.0s        0       14819303         8232.9      0.9      0.9      1.5      2.2     21.0  read

_elapsed___errors_____ops(total)___ops/sec(cum)__avg(ms)__p50(ms)__p95(ms)__p99(ms)_pMax(ms)__total
 1800.0s        0         781759          434.3      1.9      1.9      2.9      4.5     65.0  write

_elapsed___errors_____ops(total)___ops/sec(cum)__avg(ms)__p50(ms)__p95(ms)__p99(ms)_pMax(ms)__result
 1800.0s        0       15601062         8667.3      0.9      0.9      1.8      2.6     65.0

This commit pools the allocations of some objects that are created on
the simplest read path in the vectorized engine - when we have
a ColBatchScan connected with a Materializer. The benchmarks of KV95
workload showed that this improves the throughput by 0.5% or so.

Release note: None
Previously, in order to determine whether we supported the vectorization
of a flow, we would run a pretty expensive SupportsVectorized check that
performs a "fake flow setup" by actually creating all of the components
without running them. This has non-negligible performance impact on
KV-like workloads, so it has been optimized away in favor of a more
light-weight check that simply inspects the processor specs for the fact
whether the processor core can be vectorized (either natively or by
wrapping row-execution processor). All processor cores have been
audited to separate out all that we currently cannot wrap (usually
because they don't implement RowSource interface). Note that if a new
processor core is introduced and `canWrap` check is not updated, we
defensively assume that it cannot be wrapped and emit an assertion
failed error that - hopefully - should surface the fact that we need to
update the check.

Release note: None
@yuzefovich
Copy link
Member Author

Alright, the build is green, and this PR is subsumed by #55713.

@yuzefovich yuzefovich closed this Oct 28, 2020
@yuzefovich yuzefovich deleted the vec-light-check branch October 28, 2020 21:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do-not-merge bors won't merge a PR with this label.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants