-
Notifications
You must be signed in to change notification settings - Fork 44
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
Updated benchmark runner script #207
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed question after reading the PR description
cd $GPU_BDB_HOME/gpu_bdb/queries/q$qnum/ | ||
for j in $(seq 1 $N_REPEATS) | ||
do | ||
python gpu_bdb_query_$qnum\_sql.py --config_file ../../benchmark_runner/benchmark_config.yaml |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just noting that launching a process for each script will add library import-related overheads which for 30 queries quickly add up for the whole benchmark.
IIRC they add to like 30-50s
of the whole run time which is significant
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a good point, and something I had forgotten. Let's discuss this more offline
Closing as this this no longer relevant after many recent changes |
This PR:
We switched to using Python for the benchmark runner initially because we ran into errors related to library interactions, import order, and namespace visibility across nodes. At this point, we should longer need any special consideration.
There is some implicitly duplicated configuration (
INCLUDE_BLAZING
in addition to a configuration argument in thebenchmark_config.yaml
file, GPU_BDB_HOME, etc.), but this will be addressed in a follow-up that restructures a bit more broadly.