-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
[single node perf] Recalibrate and improve regression perf test #14894
Conversation
⏱️ 14h 59m total CI duration on this PR
|
testsuite/single_node_performance.py
Outdated
@@ -766,6 +811,12 @@ def print_table( | |||
if errors: | |||
print("Errors: ") | |||
print("\n".join(errors)) | |||
print("""If you expect your PR to change the performance, you need to recalibrate the values. |
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.
Thanks!
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.
Thanks!
bfe3c55
to
1ac500f
Compare
testsuite/single_node_performance.py
Outdated
@@ -766,6 +811,12 @@ def print_table( | |||
if errors: | |||
print("Errors: ") | |||
print("\n".join(errors)) | |||
print("""If you expect your PR to change the performance, you need to recalibrate the values. |
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.
Thanks!
testsuite/single_node_performance.py
Outdated
# (or if from log: | ||
# transaction_type module_working_set_size executor_type block_size expected_tps tps | ||
# ) |
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.
remove?
CALIBRATION_SEPARATOR = " " | ||
|
||
# transaction_type module_working_set_size executor_type min_ratio max_ratio median | ||
# transaction_type module_working_set_size executor_type count min_ratio max_ratio median |
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.
where does "count" come from originally again? (I read the query and it seems the binary prints it?)
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.
nvm, it's count(as="count")
1ac500f
to
4b5fbe1
Compare
4b5fbe1
to
ecaef0e
Compare
ecaef0e
to
2b7d626
Compare
@@ -17,6 +17,11 @@ on: | |||
default: false | |||
type: boolean | |||
description: Run complete version of the tests | |||
IS_MAINNET_RUN: |
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.
Is this used anywhere ?
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.
stale
IGNORE_TARGET_DETERMINATION: | ||
required: false | ||
default: false | ||
default: true |
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.
Are you sure it's a good idea to ignore by default ?
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 only for jobs triggered from the UI, right?
if we are triggering from the UI, we really shouldn't be skipping if there are no changes?
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.
Yes, you're right.
2b7d626
to
8164ee9
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
f174199
to
d07e93a
Compare
8776a21
to
390fa54
Compare
390fa54
to
955648c
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Recalibrate for RG change Update limits to be based on min_ratio / max_ratio of many runs update module working set to 100
955648c
to
419098f
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
✅ Forge suite
|
✅ Forge suite
|
✅ Forge suite
|
Recalibrate for RG change
Update limits to be based on min_ratio / max_ratio of many runs update module working set to 100
Description
How Has This Been Tested?
Key Areas to Review
Type of Change
Which Components or Systems Does This Change Impact?
Checklist