[DEMO] Sharded Import Profiler Test - #17936
Conversation
hebaalazzeh
changed the title
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds # demo comments to several package initialization files, cleans up whitespace, and quotes variables in the test script ci/run_single_test.sh. Additionally, it significantly increases the profiler's --fail-threshold from 5000 to 120000. Feedback was provided raising concerns about this threshold increase, as it could prevent the profiler from detecting actual performance regressions, and suggesting that the threshold be parameterized or lowered.
| python "${PROFILER_SCRIPT}" --package "${PACKAGE_NAME}" --iterations 11 --fail-threshold 120000 --diff-baseline "${BASELINE_CSV}" --diff-threshold 100 | ||
| else | ||
| python ${PROFILER_SCRIPT} --package ${PACKAGE_NAME} --iterations 11 --fail-threshold 5000 | ||
| python "${PROFILER_SCRIPT}" --package "${PACKAGE_NAME}" --iterations 11 --fail-threshold 120000 |
There was a problem hiding this comment.
The --fail-threshold has been significantly increased from 5000 to 120000 (a 24x increase). While this might be necessary for a demo or a specific slow test environment, such a high threshold could prevent the profiler from detecting actual performance regressions in production. Please verify if this threshold can be lowered or if it should be parameterized.
1 participant
Here is an example draft PR demonstrating the sharded import-profiler workflow running across modified packages: #17936