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

Port LLVMEngine::Optimize to skip module passes. #1553

Merged
merged 5 commits into from
Apr 21, 2021

Conversation

lmwnshn
Copy link
Contributor

@lmwnshn lmwnshn commented Apr 19, 2021

Heading

Port LLVMEngine::Optimize to skip module passes.

Description

Copies over Prashanth's commit here:
pmenon/tpl@e2d3749#diff-ed4b8c2805fa1b88ccfdd18b72ce7eebaf3485e63bf95990c7551716a9d79a5f

Unscientific timing on a separate branch showed that much
more time was spent running module passes than the singular
inlining function pass, so maybe this will help. In general, we
also would like to have function passes (as opposed to module)
wherever possible so that it is easier to time the passes.

And for a few hand-checked examples, function optimize time went up (as expected),
but overall optimize time went down.

Expect to see a speedup in the compiled tests on CI.

Copies over Prashanth's commit here:
pmenon/tpl@e2d3749#diff-ed4b8c2805fa1b88ccfdd18b72ce7eebaf3485e63bf95990c7551716a9d79a5f

Unscientific timing on a separate branch showed that much
more time was spent running module passes than function passes.
@lmwnshn lmwnshn added in-progress This PR is being actively worked on and not ready to be reviewed or merged. Mark PRs with this. ready-for-ci Indicate that this build should be run through CI. labels Apr 19, 2021
@lmwnshn lmwnshn self-assigned this Apr 19, 2021
@noisepage-checks
Copy link

Minor Decrease in Performance

Be warned: this PR may have decreased the throughput of the system slightly.

tps (%change) benchmark_type wal_device details
-0.25% tpcc RAM disk
Detailsmaster tps=22071.77, commit tps=22017.18, query_mode=extended, benchmark_type=tpcc, scale_factor=32.0000, terminals=32, client_time=60, weights={'Payment': 43, 'Delivery': 4, 'NewOrder': 45, 'StockLevel': 4, 'OrderStatus': 4}, wal_device=RAM disk, max_connection_threads=32
0.97% tpcc None
Detailsmaster tps=28897.36, commit tps=29178.04, query_mode=extended, benchmark_type=tpcc, scale_factor=32.0000, terminals=32, client_time=60, weights={'Payment': 43, 'Delivery': 4, 'NewOrder': 45, 'StockLevel': 4, 'OrderStatus': 4}, wal_device=None, max_connection_threads=32
-1.28% tpcc HDD
Detailsmaster tps=21903.6, commit tps=21623.34, query_mode=extended, benchmark_type=tpcc, scale_factor=32.0000, terminals=32, client_time=60, weights={'Payment': 43, 'Delivery': 4, 'NewOrder': 45, 'StockLevel': 4, 'OrderStatus': 4}, wal_device=HDD, max_connection_threads=32
8.64% tatp RAM disk
Detailsmaster tps=6500.35, commit tps=7062.21, query_mode=extended, benchmark_type=tatp, scale_factor=1.0000, terminals=16, client_time=60, weights={'GetAccessData': 35, 'UpdateLocation': 14, 'GetNewDestination': 10, 'GetSubscriberData': 35, 'DeleteCallForwarding': 2, 'InsertCallForwarding': 2, 'UpdateSubscriberData': 2}, wal_device=RAM disk, max_connection_threads=32
1.78% tatp None
Detailsmaster tps=7417.56, commit tps=7549.42, query_mode=extended, benchmark_type=tatp, scale_factor=1.0000, terminals=16, client_time=60, weights={'GetAccessData': 35, 'UpdateLocation': 14, 'GetNewDestination': 10, 'GetSubscriberData': 35, 'DeleteCallForwarding': 2, 'InsertCallForwarding': 2, 'UpdateSubscriberData': 2}, wal_device=None, max_connection_threads=32
7.48% tatp HDD
Detailsmaster tps=6445.35, commit tps=6927.56, query_mode=extended, benchmark_type=tatp, scale_factor=1.0000, terminals=16, client_time=60, weights={'GetAccessData': 35, 'UpdateLocation': 14, 'GetNewDestination': 10, 'GetSubscriberData': 35, 'DeleteCallForwarding': 2, 'InsertCallForwarding': 2, 'UpdateSubscriberData': 2}, wal_device=HDD, max_connection_threads=32

@noisepage-checks
Copy link

Minor Decrease in Performance

Be warned: this PR may have decreased the throughput of the system slightly.

tps (%change) benchmark_type wal_device details
-0.32% tpcc RAM disk
Detailsmaster tps=22071.77, commit tps=22000.48, query_mode=extended, benchmark_type=tpcc, scale_factor=32.0000, terminals=32, client_time=60, weights={'Payment': 43, 'Delivery': 4, 'NewOrder': 45, 'StockLevel': 4, 'OrderStatus': 4}, wal_device=RAM disk, max_connection_threads=32
2.37% tpcc None
Detailsmaster tps=28897.36, commit tps=29583.44, query_mode=extended, benchmark_type=tpcc, scale_factor=32.0000, terminals=32, client_time=60, weights={'Payment': 43, 'Delivery': 4, 'NewOrder': 45, 'StockLevel': 4, 'OrderStatus': 4}, wal_device=None, max_connection_threads=32
-2.87% tpcc HDD
Detailsmaster tps=21903.6, commit tps=21275.94, query_mode=extended, benchmark_type=tpcc, scale_factor=32.0000, terminals=32, client_time=60, weights={'Payment': 43, 'Delivery': 4, 'NewOrder': 45, 'StockLevel': 4, 'OrderStatus': 4}, wal_device=HDD, max_connection_threads=32
-2.16% tatp RAM disk
Detailsmaster tps=6500.35, commit tps=6359.76, query_mode=extended, benchmark_type=tatp, scale_factor=1.0000, terminals=16, client_time=60, weights={'GetAccessData': 35, 'UpdateLocation': 14, 'GetNewDestination': 10, 'GetSubscriberData': 35, 'DeleteCallForwarding': 2, 'InsertCallForwarding': 2, 'UpdateSubscriberData': 2}, wal_device=RAM disk, max_connection_threads=32
0.35% tatp None
Detailsmaster tps=7417.56, commit tps=7443.18, query_mode=extended, benchmark_type=tatp, scale_factor=1.0000, terminals=16, client_time=60, weights={'GetAccessData': 35, 'UpdateLocation': 14, 'GetNewDestination': 10, 'GetSubscriberData': 35, 'DeleteCallForwarding': 2, 'InsertCallForwarding': 2, 'UpdateSubscriberData': 2}, wal_device=None, max_connection_threads=32
5.18% tatp HDD
Detailsmaster tps=6445.35, commit tps=6779.44, query_mode=extended, benchmark_type=tatp, scale_factor=1.0000, terminals=16, client_time=60, weights={'GetAccessData': 35, 'UpdateLocation': 14, 'GetNewDestination': 10, 'GetSubscriberData': 35, 'DeleteCallForwarding': 2, 'InsertCallForwarding': 2, 'UpdateSubscriberData': 2}, wal_device=HDD, max_connection_threads=32

@codecov
Copy link

codecov bot commented Apr 20, 2021

Codecov Report

❗ No coverage uploaded for pull request base (master@3d88fec). Click here to learn what that means.
The diff coverage is 59.44%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #1553   +/-   ##
=========================================
  Coverage          ?   82.37%           
=========================================
  Files             ?      723           
  Lines             ?    51352           
  Branches          ?        0           
=========================================
  Hits              ?    42300           
  Misses            ?     9052           
  Partials          ?        0           
Impacted Files Coverage Δ
script/self_driving/forecasting/forecaster.py 93.54% <ø> (ø)
...c/include/execution/compiler/compilation_context.h 100.00% <ø> (ø)
src/include/execution/compiler/executable_query.h 90.90% <0.00%> (ø)
src/include/metrics/abstract_raw_data.h 50.00% <0.00%> (ø)
...clude/self_driving/forecasting/workload_forecast.h 0.00% <0.00%> (ø)
...lf_driving/forecasting/workload_forecast_segment.h 0.00% <0.00%> (ø)
src/include/self_driving/planning/pilot.h 0.00% <ø> (ø)
src/include/self_driving/planning/pilot_thread.h 0.00% <0.00%> (ø)
src/include/settings/settings_param.h 100.00% <ø> (ø)
src/self_driving/forecasting/workload_forecast.cpp 0.00% <0.00%> (ø)
... and 30 more

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 3d88fec...1015122. Read the comment docs.

@lmwnshn lmwnshn added ready-for-review This PR passes all checks and is ready to be reviewed. Mark PRs with this. and removed in-progress This PR is being actively worked on and not ready to be reviewed or merged. Mark PRs with this. labels Apr 20, 2021
@turingcompl33t turingcompl33t self-requested a review April 20, 2021 17:47
Copy link
Contributor

@turingcompl33t turingcompl33t left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. One small nit. Excited to see if the changes to compilation time materialize!

src/execution/vm/llvm_engine.cpp Outdated Show resolved Hide resolved
src/execution/vm/llvm_engine.cpp Outdated Show resolved Hide resolved
src/execution/vm/llvm_engine.cpp Show resolved Hide resolved
@lmwnshn
Copy link
Contributor Author

lmwnshn commented Apr 20, 2021

I kept the variables local to the Optimize() function because it isn't relevant to the rest of the CompiledModuleBuilder and I don't think we're exposing this as a tunable; if we do we will probably add it as a parameter to the Optimize() function anyway.

Copy link
Contributor

@turingcompl33t turingcompl33t left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updating my review as comments have been addressed.

@noisepage-checks
Copy link

Minor Decrease in Performance

Be warned: this PR may have decreased the throughput of the system slightly.

tps (%change) benchmark_type wal_device details
-4.01% tpcc RAM disk
Detailsmaster tps=22123.02, commit tps=21236.1, query_mode=extended, benchmark_type=tpcc, scale_factor=32.0000, terminals=32, client_time=60, weights={'Payment': 43, 'Delivery': 4, 'NewOrder': 45, 'StockLevel': 4, 'OrderStatus': 4}, wal_device=RAM disk, max_connection_threads=32
0.43% tpcc None
Detailsmaster tps=28404.62, commit tps=28528.0, query_mode=extended, benchmark_type=tpcc, scale_factor=32.0000, terminals=32, client_time=60, weights={'Payment': 43, 'Delivery': 4, 'NewOrder': 45, 'StockLevel': 4, 'OrderStatus': 4}, wal_device=None, max_connection_threads=32
-0.97% tpcc HDD
Detailsmaster tps=21785.4, commit tps=21574.58, query_mode=extended, benchmark_type=tpcc, scale_factor=32.0000, terminals=32, client_time=60, weights={'Payment': 43, 'Delivery': 4, 'NewOrder': 45, 'StockLevel': 4, 'OrderStatus': 4}, wal_device=HDD, max_connection_threads=32
6.61% tatp RAM disk
Detailsmaster tps=6525.4, commit tps=6956.55, query_mode=extended, benchmark_type=tatp, scale_factor=1.0000, terminals=16, client_time=60, weights={'GetAccessData': 35, 'UpdateLocation': 14, 'GetNewDestination': 10, 'GetSubscriberData': 35, 'DeleteCallForwarding': 2, 'InsertCallForwarding': 2, 'UpdateSubscriberData': 2}, wal_device=RAM disk, max_connection_threads=32
-0.47% tatp None
Detailsmaster tps=7517.47, commit tps=7482.41, query_mode=extended, benchmark_type=tatp, scale_factor=1.0000, terminals=16, client_time=60, weights={'GetAccessData': 35, 'UpdateLocation': 14, 'GetNewDestination': 10, 'GetSubscriberData': 35, 'DeleteCallForwarding': 2, 'InsertCallForwarding': 2, 'UpdateSubscriberData': 2}, wal_device=None, max_connection_threads=32
7.7% tatp HDD
Detailsmaster tps=6385.94, commit tps=6877.73, query_mode=extended, benchmark_type=tatp, scale_factor=1.0000, terminals=16, client_time=60, weights={'GetAccessData': 35, 'UpdateLocation': 14, 'GetNewDestination': 10, 'GetSubscriberData': 35, 'DeleteCallForwarding': 2, 'InsertCallForwarding': 2, 'UpdateSubscriberData': 2}, wal_device=HDD, max_connection_threads=32

@lmwnshn lmwnshn added ready-to-merge This PR is ready to be merged. Mark PRs with this. and removed ready-for-review This PR passes all checks and is ready to be reviewed. Mark PRs with this. labels Apr 20, 2021
@noisepage-checks
Copy link

Minor Decrease in Performance

Be warned: this PR may have decreased the throughput of the system slightly.

tps (%change) benchmark_type wal_device details
0.01% tpcc RAM disk
Detailsmaster tps=22123.02, commit tps=22124.74, query_mode=extended, benchmark_type=tpcc, scale_factor=32.0000, terminals=32, client_time=60, weights={'Payment': 43, 'Delivery': 4, 'NewOrder': 45, 'StockLevel': 4, 'OrderStatus': 4}, wal_device=RAM disk, max_connection_threads=32
3.98% tpcc None
Detailsmaster tps=28404.62, commit tps=29534.67, query_mode=extended, benchmark_type=tpcc, scale_factor=32.0000, terminals=32, client_time=60, weights={'Payment': 43, 'Delivery': 4, 'NewOrder': 45, 'StockLevel': 4, 'OrderStatus': 4}, wal_device=None, max_connection_threads=32
-1.14% tpcc HDD
Detailsmaster tps=21785.4, commit tps=21536.48, query_mode=extended, benchmark_type=tpcc, scale_factor=32.0000, terminals=32, client_time=60, weights={'Payment': 43, 'Delivery': 4, 'NewOrder': 45, 'StockLevel': 4, 'OrderStatus': 4}, wal_device=HDD, max_connection_threads=32
5.94% tatp RAM disk
Detailsmaster tps=6525.4, commit tps=6913.05, query_mode=extended, benchmark_type=tatp, scale_factor=1.0000, terminals=16, client_time=60, weights={'GetAccessData': 35, 'UpdateLocation': 14, 'GetNewDestination': 10, 'GetSubscriberData': 35, 'DeleteCallForwarding': 2, 'InsertCallForwarding': 2, 'UpdateSubscriberData': 2}, wal_device=RAM disk, max_connection_threads=32
-2.33% tatp None
Detailsmaster tps=7517.47, commit tps=7342.01, query_mode=extended, benchmark_type=tatp, scale_factor=1.0000, terminals=16, client_time=60, weights={'GetAccessData': 35, 'UpdateLocation': 14, 'GetNewDestination': 10, 'GetSubscriberData': 35, 'DeleteCallForwarding': 2, 'InsertCallForwarding': 2, 'UpdateSubscriberData': 2}, wal_device=None, max_connection_threads=32
5.47% tatp HDD
Detailsmaster tps=6385.94, commit tps=6735.3, query_mode=extended, benchmark_type=tatp, scale_factor=1.0000, terminals=16, client_time=60, weights={'GetAccessData': 35, 'UpdateLocation': 14, 'GetNewDestination': 10, 'GetSubscriberData': 35, 'DeleteCallForwarding': 2, 'InsertCallForwarding': 2, 'UpdateSubscriberData': 2}, wal_device=HDD, max_connection_threads=32

1 similar comment
@noisepage-checks
Copy link

Minor Decrease in Performance

Be warned: this PR may have decreased the throughput of the system slightly.

tps (%change) benchmark_type wal_device details
0.01% tpcc RAM disk
Detailsmaster tps=22123.02, commit tps=22124.74, query_mode=extended, benchmark_type=tpcc, scale_factor=32.0000, terminals=32, client_time=60, weights={'Payment': 43, 'Delivery': 4, 'NewOrder': 45, 'StockLevel': 4, 'OrderStatus': 4}, wal_device=RAM disk, max_connection_threads=32
3.98% tpcc None
Detailsmaster tps=28404.62, commit tps=29534.67, query_mode=extended, benchmark_type=tpcc, scale_factor=32.0000, terminals=32, client_time=60, weights={'Payment': 43, 'Delivery': 4, 'NewOrder': 45, 'StockLevel': 4, 'OrderStatus': 4}, wal_device=None, max_connection_threads=32
-1.14% tpcc HDD
Detailsmaster tps=21785.4, commit tps=21536.48, query_mode=extended, benchmark_type=tpcc, scale_factor=32.0000, terminals=32, client_time=60, weights={'Payment': 43, 'Delivery': 4, 'NewOrder': 45, 'StockLevel': 4, 'OrderStatus': 4}, wal_device=HDD, max_connection_threads=32
5.94% tatp RAM disk
Detailsmaster tps=6525.4, commit tps=6913.05, query_mode=extended, benchmark_type=tatp, scale_factor=1.0000, terminals=16, client_time=60, weights={'GetAccessData': 35, 'UpdateLocation': 14, 'GetNewDestination': 10, 'GetSubscriberData': 35, 'DeleteCallForwarding': 2, 'InsertCallForwarding': 2, 'UpdateSubscriberData': 2}, wal_device=RAM disk, max_connection_threads=32
-2.33% tatp None
Detailsmaster tps=7517.47, commit tps=7342.01, query_mode=extended, benchmark_type=tatp, scale_factor=1.0000, terminals=16, client_time=60, weights={'GetAccessData': 35, 'UpdateLocation': 14, 'GetNewDestination': 10, 'GetSubscriberData': 35, 'DeleteCallForwarding': 2, 'InsertCallForwarding': 2, 'UpdateSubscriberData': 2}, wal_device=None, max_connection_threads=32
5.47% tatp HDD
Detailsmaster tps=6385.94, commit tps=6735.3, query_mode=extended, benchmark_type=tatp, scale_factor=1.0000, terminals=16, client_time=60, weights={'GetAccessData': 35, 'UpdateLocation': 14, 'GetNewDestination': 10, 'GetSubscriberData': 35, 'DeleteCallForwarding': 2, 'InsertCallForwarding': 2, 'UpdateSubscriberData': 2}, wal_device=HDD, max_connection_threads=32

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
ready-for-ci Indicate that this build should be run through CI. ready-to-merge This PR is ready to be merged. Mark PRs with this.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants