-
Notifications
You must be signed in to change notification settings - Fork 660
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
[api] Use folk java process to avoid jvm consume GPU memory #2882
Conversation
return; | ||
} | ||
int cudaVersion = getCudaVersion(); | ||
System.out.println(gpuCount + "," + cudaVersion); |
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.
Why use println?
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.
We need to read the console output from java side
try { | ||
testCudaUtils(); | ||
} finally { | ||
System.clearProperty("ai.djl.util.folk_cuda_utils"); |
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.
System.clearProperty("ai.djl.util.folk_cuda_utils"); | |
System.clearProperty("ai.djl.util.cuda.folk"); |
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.
Fixed
@@ -50,6 +54,11 @@ public static boolean hasCuda() { | |||
* @return the number of GPUs available in the system | |||
*/ | |||
public static int getGpuCount() { | |||
if (Boolean.getBoolean("ai.djl.util.cuda.folk")) { | |||
String[] ret = execute(-1); |
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.
Can we cache the result of execute?
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.
Fixed
Codecov ReportAttention:
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## master #2882 +/- ##
============================================
+ Coverage 72.08% 72.28% +0.19%
- Complexity 5126 7177 +2051
============================================
Files 473 708 +235
Lines 21970 31988 +10018
Branches 2351 3332 +981
============================================
+ Hits 15838 23121 +7283
- Misses 4925 7276 +2351
- Partials 1207 1591 +384 ☔ View full report in Codecov by Sentry. |
Description
Brief description of what this PR is about