IGNITE-20688 Fixed broken handles after binary object detached (lightweighted). #11292
Conversation
petrov-mg
force-pushed
the
IGNITE-20688-light-detach
branch
from
5f0196a
to
cc967c4
Compare
petrov-mg
force-pushed
the
IGNITE-20688-light-detach
branch
from
cc967c4
to
0a38c74
Compare
| } | ||
|
|
||
| /** */ | ||
| public byte[] readObject() { |
There was a problem hiding this comment.
Used only by test. Perhaps, can be replaced in test with copyObject() or, at least, can be package-private.
| skipBytes(Byte.BYTES); // String type. | ||
|
|
||
| skipBytes(in.readInt()); |
| @@ -282,16 +284,33 @@ private byte[] valueBytesFromArray(CacheObjectValueContext ctx) { | |||
| * @return Detached binary object. | |||
| */ | |||
| public BinaryObjectImpl detach() { | |||
| return detach(true); | |||
There was a problem hiding this comment.
Why resolveCrossObjectReferences is true by default? Looks like it can be false.
| import org.apache.ignite.internal.binary.streams.BinaryOutputStream; | ||
|
|
||
| /** */ | ||
| public class RawBytesObjectReader implements BinaryPositionReadable { |
There was a problem hiding this comment.
Perhaps it's worth to add Binary word to class name. And we don't read objects in this class, so object readed is not very good name also. Perhaps extractor or something like this.
|
|
||
| /** */ | ||
| private void overrideHash(BinaryOutputStream writer, int writerRootObjStartPos, ObjectDescriptor objDesc) { | ||
| int hashCode = instance().hashCode( |
There was a problem hiding this comment.
Let's use method name with class name for instance() (without static import), or instance() can be readed as local class method.
|
|
||
| /** {@inheritDoc} */ | ||
| @Override public int compareTo(@NotNull ObjectDetachHelper.ObjectDescriptor other) { | ||
| return Integer.compare(objStartOffset, other.objDataStartOffset); |
There was a problem hiding this comment.
Why different fields are compared?
| } | ||
|
|
||
| /** */ | ||
| public BinaryObjectImpl detach(boolean resolveCrossObjectReferences) { |
There was a problem hiding this comment.
Abbreviation should be used for Object
| BinaryReaderHandles hnds = new BinaryReaderHandles(); | ||
|
|
||
| return (T)unwrapBinary(marsh.deserialize(in, hnds), hnds, clazz); | ||
| return (T)unwrapBinary(marsh.deserialize(in, new BinaryReaderHandles()), clazz); |
There was a problem hiding this comment.
One line statement with braces
|
|
||
| /** */ | ||
| @RunWith(Parameterized.class) | ||
| @WithSystemProperty(key = IGNITE_BINARY_SORT_OBJECT_FIELDS, value = "true") |
There was a problem hiding this comment.
BinaryUtils.FIELDS_SORTED_ORDER is a static field and initiated when BinaryUtils class is initiated. If current JVM run some other tests before (on TC, for example) field is not reinitiated with this system property.
Thank you for submitting the pull request to the Apache Ignite.
In order to streamline the review of the contribution
we ask you to ensure the following steps have been taken:
The Contribution Checklist
The description explains WHAT and WHY was made instead of HOW.
The following pattern must be used:
IGNITE-XXXX Change summarywhereXXXX- number of JIRA issue.(see the Maintainers list)
the
green visaattached to the JIRA ticket (see TC.Bot: Check PR)Notes
If you need any help, please email dev@ignite.apache.org or ask anу advice on http://asf.slack.com #ignite channel.