-
Notifications
You must be signed in to change notification settings - Fork 5.3k
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
[Java] Fix the issue that the cached value in RayObject
is serialized
#7613
Conversation
Can one of the admins verify this patch? |
Test FAILed. |
Does it make sense to add a test for serialization? |
@ericl I thought about adding a test. But this PR is more like a performance improvement, rather than a bug fix. If we want to add a test, I can only assert the size of the serialized object is small. But this seems weird. So I didn't do that. |
Fair enough, I guess you can always assume T is serializable as well.
…On Mon, Mar 16, 2020, 12:05 AM Hao Chen ***@***.***> wrote:
@ericl <https://github.com/ericl> I thought about adding a test. But this
PR is more like a performance improvement, rather than a bug fix. If we
want to add a test, I can only assert the size of the serialized object is
small. But this seems weird. So I didn't do that.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#7613 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAADUSVXG5LA6QGBT55JI33RHXFUDANCNFSM4LLXIQHA>
.
|
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.
LGTM
Test PASSed. |
* enable * Turn on eager eviction * Shorten tests and drain ReferenceCounter * Don't force kill actor handles that have gone out of scope, lint * Fix locks * Cleanup Plasma Async Callback (#7452) * [rllib][tune] fix some nans (#7611) * Change /tmp to platform-specific temporary directory (#7529) * [Serve] UI Improvements (#7569) * bugfix about test_dynres.py (#7615) Co-authored-by: senlin.zsl <senlin.zsl@antfin.com> * Java call Python actor method use actor.call (#7614) * bug fix about useage of absl::flat_hash_map::erase and absl::flat_hash_set::erase (#7633) Co-authored-by: senlin.zsl <senlin.zsl@antfin.com> * [Java] Make both `RayActor` and `RayPyActor` inheriting from `BaseActor` (#7462) * [Java] Fix the issue that the cached value in `RayObject` is serialized (#7613) * Add failure tests to test_reference_counting (#7400) * Fix typo in asyncio documentation (#7602) * Fix segfault * debug * Force kill actor * Fix test
Why are these changes needed?
No need to serialize the cached value, just serialize the id.
Related issue number
Checks
scripts/format.sh
to lint the changes in this PR.