Additional improvements to new marshal logic#8685
Conversation
This addresses some of the concerns mentioned in jruby#8683. * ThreadContext propagated everywhere it might end up being needed * Consistent ordering of new context and out arguments * No longer save the context in RubyOutputStream; use a new set of write methods.
|
@enebo I have implemented the context-propagation part of this, which did find a few places that were forced to re-acquire the context. However I now feel it is excessive to have a special RubyOutputStream that can propagate the context solely for the purpose of raising an error. I'd like your thoughts on how to reduce or eliminate RubyOutputStream, since all it does is raise Ruby errors and drop |
|
@headius yeah let's chat about this when we are both around. |
Unfortunately deprecating code does not avoid removal warnings, so
we also have to SuppressWarnings("removal") anywhere we reference
MarshalStream, and cannot directly import the class.
* NewMarshal => Dumper * NewMarshalCache inlined (no need for separate object) * RubyOutputStream drops context signatures and moves to org.jruby.util.io (general OutputStream wrapper utility)
headius
force-pushed
the
more_new_marshal
branch
from
30ca93f to
d0a0c21
Compare
|
Interface mitigation has been made moot by our intent to fully commit to the new API. As described in #8683 (comment) I could find no examples of ObjectMarshal being implemented outside of JRuby, so we just deprecate the old methods and classes for removal and will aid users in supporting the new API. |
These writes don't need context now that RubyOutputStream does not receive context.
* Use switch expression for simple switch that assigns a var. * Remove cast to unchecked RubyArray.
2 participants
This PR is intended to address the concerns mentioned in #8683.
Fixes #8683.
Goals: