[JavaWorker]Changes to the directory under src for support java worker #2093
Conversation
-------------------------- This commit includes changes to the directory under src, which is part of the java worker support of Ray. It consists of the following changes: src/common/task.cc - just fix null point problem org_ray_spi_impl_DefaultLocalSchedulerClient.* - JNI support for local scheduler client, and the org_ray_spi_impl_DefaultLocalSchedulerClient.cc file is not autogenerated
|
Test PASSed. |
|
Test PASSed. |
salah-man
force-pushed
the
java_worker_mr2
branch
from
0a49eee to
9218378
Compare
|
Test PASSed. |
src/common/task.cc
Outdated
| if (ids == nullptr) | ||
| return 0; | ||
| else | ||
| return ids->size(); |
There was a problem hiding this comment.
The linting doesn't seem to be enforcing this, but please change this to
if (ids == nullptr) {
return 0;
} else {
return ids->size();
}There was a problem hiding this comment.
Got it, thanks for your time, i will change them soon
| @@ -0,0 +1,224 @@ | |||
|
|
|||
There was a problem hiding this comment.
remove newline at beginning
| _bytes = wid; | ||
|
|
||
| jbyte *b = (jbyte *) _env->GetByteArrayElements(_bytes, NULL); | ||
| PID = (UniqueID *) b; |
There was a problem hiding this comment.
In this file, let's remove all C style casts and instead use static_cast
|
Test PASSed. |
salah-man
force-pushed
the
java_worker_mr2
branch
from
aeb7000 to
268b31a
Compare
|
Test FAILed. |
|
Test PASSed. |
|
Excuse me, but i just fix the problem as the comment before, could you review this pr again? Thank you very much. @robertnishihara |
| const char *nativeString = env->GetStringUTFChars(sockName, JNI_FALSE); | ||
| auto client = LocalSchedulerConnection_init(nativeString, *w.PID, isWorker); | ||
| env->ReleaseStringUTFChars(sockName, nativeString); | ||
| return (jlong) client; |
| * Signature: (J)[B | ||
| */ | ||
| JNIEXPORT jbyteArray JNICALL | ||
| Java_org_ray_spi_impl_DefaultLocalSchedulerClient__1getTaskTodo(JNIEnv *env, |
There was a problem hiding this comment.
I'm a bit confused about where these function names come from. E.g., what does the 1 mean and the Todo?
There was a problem hiding this comment.
Oh, it is auto generated by javah, which make the function name looks stange.
| TaskSpec *task = | ||
| reinterpret_cast<char *>(env->GetDirectBufferAddress(buff)) + pos; | ||
| std::vector<ObjectID> execution_dependencies; | ||
| if (cursorId != NULL) { |
There was a problem hiding this comment.
In general, we should avoid using NULL and instead use nullptr for pointers. However, I haven't worked much with JNI so should it be different in this case?
There was a problem hiding this comment.
I just fix them, it is pointer here
| jlong numGpus) { | ||
| // native private static long _init(String localSchedulerSocket, | ||
| // byte[] workerId, byte[] actorId, boolean isWorker, long numGpus); | ||
| UniqueIdFromJByteArray w(env, wid), a(env, actorId); |
There was a problem hiding this comment.
In general I think we should be using much more informative variable names (e.g., full words) and avoiding abbreviations
| jbyteArray result; | ||
| result = env->NewByteArray(task_size); | ||
| if (result == NULL) { | ||
| return NULL; /* out of memory error thrown */ |
There was a problem hiding this comment.
Is the plan to let the caller check for this and throw a Java exception?
There was a problem hiding this comment.
yes, currently this situation will throw a Exception in java
|
Test FAILed. |
salah-man
force-pushed
the
java_worker_mr2
branch
from
17d3dd5 to
1e0d4fc
Compare
|
Test FAILed. |
|
Test PASSed. |
salah-man
force-pushed
the
java_worker_mr2
branch
from
fbe9e25 to
19dab9e
Compare
|
Test PASSed. |
* master: [autoscaler] GCP node provider (ray-project#2061) [xray] Evict tasks from the lineage cache (ray-project#2152) [ASV] Add ray.init and simple Ray benchmarks (ray-project#2166) Re-encrypt key for uploading to S3 from travis to use travis-ci.com. (ray-project#2169) [rllib] Fix A3C PyTorch implementation (ray-project#2036) [JavaWorker] Do not kill local-scheduler-forked workers in RunManager.cleanup (ray-project#2151) Update Travis CI badge from travis-ci.org to travis-ci.com. (ray-project#2155) Implement Python global state API for xray. (ray-project#2125) [xray] Improve flush algorithm for the lineage cache (ray-project#2130) Fix support for actor classmethods (ray-project#2146) Add empty df test (ray-project#1879) [JavaWorker] Enable java worker support (ray-project#2094) [DataFrame] Fixing the code formatting of the tests (ray-project#2123) Update resource documentation (remove outdated limitations). (ray-project#2022) bugfix: use array redis_primary_addr out of its scope (ray-project#2139) Fix infinite retry in Push function. (ray-project#2133) [JavaWorker] Changes to the directory under src for support java worker (ray-project#2093) Integrate credis with Ray & route task table entries into credis. (ray-project#1841)
This commit includes changes to the directory under src, which is part of the java worker support of Ray.
It consists of the following changes:
src/common/task.cc - just fix null point problem
org_ray_spi_impl_DefaultLocalSchedulerClient.* - JNI support for local scheduler client, and the org_ray_spi_impl_DefaultLocalSchedulerClient.cc file is not autogenerated