[JNI] Adds retryCount to RmmEventHandler.onAllocFailure - #11940
Conversation
abellina
added
Java
abellina
added
non-breaking
| // this should not be called since it was the previous interface, | ||
| // and it was abstract before. | ||
| return false; |
There was a problem hiding this comment.
Since it should not be called, we may throw an exception instead, so we can know that there was something wrongly called and reached here.
There was a problem hiding this comment.
Sure, I can make this throw. Note that this is going away very soon, it is mainly to give us a few days while we update the rest of the code. The end result is that we'll remove these default implementations, and callers must use the non-deprecated code.
| // newer code should override this implementation of `onAllocFailure` to handle | ||
| // the `isRetry` flag. Otherwise, we call the prior implementation to not | ||
| // break existing code. | ||
| return onAllocFailure(sizeRequested); |
There was a problem hiding this comment.
This seems to just call the function above (which returns false all the time)?
There was a problem hiding this comment.
If you have code that implemented onAllocFailure (all existing code), no it just calls that impl. So this should be clearer given an exception in the deprecated code perhaps.
abellina
changed the title
jlowe
left a comment
jlowe
left a comment
There was a problem hiding this comment.
Comment nit but otherwise lgtm.
Co-authored-by: Jason Lowe <jlowe@nvidia.com>
Codecov ReportBase: 87.40% // Head: 86.87% // Decreases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## branch-22.12 #11940 +/- ##
================================================
- Coverage 87.40% 86.87% -0.54%
================================================
Files 133 133
Lines 21833 21905 +72
================================================
- Hits 19084 19030 -54
- Misses 2749 2875 +126
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
|
@gpucibot merge |
3 participants
This adds the method
boolean onAllocFailure(long sizeRequested, int retryCount)toRmmEventHandler, to help handling code keep track of the number of times an allocation failure has been retried.With this code callers can perform extra logic that depends on whether the callback was due to a brand new allocation failure, or one that has failed in the past and is being retried.
This will be used here: NVIDIA/cudf-spark#6768