Fix JFR profile summary skip check#15997
Conversation
Signed-off-by: prithvi <prithvisivasankar@gmail.com>
|
don't have permissions to add skip-changelog, please someone help adding. Thanks! |
Signed-off-by: prithvi <prithvisivasankar@gmail.com>
mikemccand
left a comment
There was a problem hiding this comment.
Thank you @iprithv! I'll merge soon -- this is a nice discovery of pre-existing issue making -Ptests.profile=true not work (in some cases? maybe on fully clean checkout it'd work?).
|
|
||
| private static boolean taskDidNotFail(Test task) { | ||
| try { | ||
| task.getState().rethrowFailure(); |
There was a problem hiding this comment.
The null check was insufficient?
There was a problem hiding this comment.
I checked w/ Gemini (I was worried maybe this method had side effects or so -- it's fine (idempotent)): https://share.google/aimode/keVKF6K1aIoSshuoI
There was a problem hiding this comment.
Yes @mikemccand. Logically it was enough, but Error Prone flagged getFailure() == null as RedundantNullCheck in build-infra-shadow:compileJava. rethrowFailure() avoids that and keeps the same intent. Thanks for checking it is idempotent!
|
I'll backport both changes to 10.x. |
|
This change worked -- I'm able to simply run profiling: but I still get not-really-loving top CPU stacks: What is so heavily using |
|
OK nevermind if I lower the stack size, the profiling looks more reasonable -- I think it's working! -- I'm kinda happy that |
Hmm actually this is tricky -- in 10.x the profiling gradle integration is different (in |
* Fix JFR profile summary skip check Signed-off-by: prithvi <prithvisivasankar@gmail.com> * Fix JFR profile summary skip check Signed-off-by: prithvi <prithvisivasankar@gmail.com> --------- Signed-off-by: prithvi <prithvisivasankar@gmail.com>
Description
fixes #15983 (comment)
showJfrProfileSummaryusedTest.getState().getDidWork()to decide whether any test task failed. This can be false for skipped/up-to-date/not-selected tasks even when nothing failed, causing the profile summary to be skipped incorrectly.Used
Test.getState().getFailure() == nullinstead so the profile summary is skipped only when a test task actually failed.