Hello, Igniters! I've developed Externalizable interface support using BinaryMarshaller [1].
I have a misunderstanding with defining BinaryWriteMode in BinaryUtils.mode(cls): there is AffinityKey class which implements Externalizable and registered with ReflectiveSerialize, BinaryMarshaller defines it as BinaryWriteMode.OBJ and uses reflection according to current logic. I want to say that AffinityKey must be defined as BinaryWriteMode.OBJ although the class implements the Externalizable interface. I have to add a new one more parameter in BinaryUtils.mode(cls) to define BinaryWriteMode in a proper way. Could you please review and comment my solution [2]? BTW, I have benchmarked my solution by GridMarshallerPerformanceTest and it becomes faster up to 2 times (GridMarshaller).My JMH tests show that marshal faster up to 50% and unmarshal faster up to 100% on an Externalizable object. Also, I've filed an issue for Serializable interface support using BinaryMarshaller [3] as it has been described earlier. [1] https://issues.apache.org/jira/browse/IGNITE-2894 [2] https://reviews.ignite.apache.org/ignite/review/IGNT-CR-278 [3] https://issues.apache.org/jira/browse/IGNITE-6172 2017-08-21 20:43 GMT+03:00 Valentin Kulichenko < [email protected]>: > Nikita, > > I think anything binary related should have higher priority than > Externalizable. I.e. if user explicitly implemented Binarylizable or > provided a BinarySerializer, then BinaryMarshaller should of course use > that and ignore Externalizable. > > -Val > > On Mon, Aug 21, 2017 at 9:29 AM, Nikita Amelchev <[email protected]> > wrote: > > > Hello, Igniters. > > > > I am developing Externalizable interface support by BinaryMarshaller > > through new type constant. BinaryMarshaller allows using BinarySerializer > > to manage serialization. I need to define BinaryWriteMode in the > > BinaryClassDescriptor constructor. In case of the Binarylizable > interface - > > serializer is ignored and BinaryWriteMode is BINARY. Can I do the same > with > > the Externalizable interface? > > > > In this case, I have issues with AffinityKey: some tests have failed > > because of they except serialization logic of defined the serializer > > instead of Externalizable logic. What is the priority between predefined > > BinarySerializer for class and implementation of Externalizable > interface? > > > > 2017-08-01 13:09 GMT+03:00 Vladimir Ozerov <[email protected]>: > > > > > Valya, > > > It makes sense to have both Externalizable and Binarylizable, as user > > might > > > want to serialize object for different systems. E.g. deserialize binary > > > stream from Kafka in Externalizable mode, and then put it to Ignite > with > > > Binarylizable to allow for field access without deserialization. > > > > > > Nikita, > > > I think that Externalizable should be written in the same way as we > write > > > fields in "raw" mode. So may be it will be enough to simply implement > our > > > own ObjectOutput interface on top of existing BinaryWriterExImpl. Makes > > > sense? > > > > > > Vladimir. > > > > > > On Thu, Jun 22, 2017 at 1:30 AM, Valentin Kulichenko < > > > [email protected]> wrote: > > > > > > > Hi Nikita, > > > > > > > > 1. Makes sense to me. > > > > > > > > 2. Externalizable object should not be written as binary with flag > 103, > > > it > > > > should be written in the same way it's written now. I don't see any > > > reason > > > > to change the protocol. Purpose of this task it to move the logic to > > > binary > > > > marshaller instead of depending on optimized marshaller, and also > fully > > > > support handles for these objects and objects included in them. > > Currently > > > > binary marshaller and optimized marshaller use different set of > > handles - > > > > this is the main downside of current implementation. > > > > > > > > 3. I think this order is correct, but does it even make sense to > > > implement > > > > both Binarylizable and Externalizable? > > > > > > > > -Val > > > > > > > > On Mon, Jun 19, 2017 at 8:58 AM, Nikita Amelchev < > [email protected] > > > > > > > wrote: > > > > > > > > > Hello everebody. > > > > > > > > > > I would like to clarify about some moments in marshaller about > custom > > > > > serialization. > > > > > > > > > > 1. I suggest to divide the issue into two tasks: support the > > > > Externalizable > > > > > and support the Serializable. The second task is to do as a > separate > > > > issue. > > > > > > > > > > 2. In case the Optimized marshaller when object is the > Extenalizable > > > > > BinaryUtils.unmarshal() return deserialize value. But if we will > not > > > use > > > > > Optimized marshaller and write the Extenalizable as the Object(103) > > it > > > > > return the BinaryObjectExImpl. It break testBuilderExternalizable. > > (If > > > we > > > > > replace Externalizable to Binarilylizable it also dont work). Fix - > > > check > > > > > that object is the Extenalizable and deserialize > > > > > manual(BinaryUtils.java:1833 in PR). We will use this fix or return > > > > > BinaryObjectExImpl? > > > > > > > > > > 3. What are priority if was implemented several interfaces: > > > Binarylizable > > > > > -> Externalizable -> Serializable ? > > > > > > > > > > Also can you pre review this issue? > > > > > PR: https://github.com/apache/ignite/pull/2160 > > > > > > > > > > 2017-04-18 17:41 GMT+03:00 Valentin Kulichenko < > > > > > [email protected]>: > > > > > > > > > > > Nikita, > > > > > > > > > > > > For Externalizable option 1 is the correct one. Externalizable > > > objects > > > > > > should not be treated as binary objects. > > > > > > > > > > > > For read/writeObject, you indeed have to extend > ObjectOutputStream. > > > > > > writeObject() is final because you should extend > > > writeObjectOverride() > > > > > > instead. Take a look at ObjectOutputStream's JavaDoc and on how > > this > > > is > > > > > > done in OptimizedObjectOutputStream. Note that ideally we need to > > > > > implement > > > > > > everything that is included in Java serialization spec, including > > > some > > > > > > non-trivial stuff like PutField. I would check if it's possible > to > > > > > somehow > > > > > > reuse the code that already exists in optimized marshaller as > much > > as > > > > > > possible. > > > > > > > > > > > > -Val > > > > > > > > > > > > On Tue, Apr 18, 2017 at 1:36 PM, Nikita Amelchev < > > > [email protected] > > > > > > > > > > > wrote: > > > > > > > > > > > > > I see two ways to support the Externalizable in the BM: > > > > > > > 1. Add a new type constant to the GridBinaryMarshaller class > etc > > > and > > > > > > > read/writeExternal in the BinaryClassDescriptor. > > > > > > > 2. Make read/writeExternal through the BINARY type without > > updating > > > > > > > metadata. > > > > > > > I don't know how to make a support read/writeObject of the > > > > Serializable > > > > > > > without delegating to the OM. Because read/writeObject methods > > need > > > > the > > > > > > > Objectoutputstream class argument. One way is to delegate it to > > the > > > > > > > OptimizedObjectOutputStream. Second way is to extend the > > > > > > Objectoutputstream > > > > > > > in the BinaryWriterExImpl. But it is wrong way because the > > > > writeObject > > > > > is > > > > > > > final. > > > > > > > > > > > > > > 2017-01-19 20:46 GMT+03:00 Valentin Kulichenko < > > > > > > > [email protected]>: > > > > > > > > > > > > > > > Nikita, > > > > > > > > > > > > > > > > In my view we just need to support Externalizable and > > > > > > > > writeObject/readObject in BinaryMarshaller and get rid of > > > > delegation > > > > > to > > > > > > > > optimized marshaller. Once such classes also go through > > > > > > BinaryMarshaller > > > > > > > > streams, they will be aware of binary configuration and will > > > share > > > > > the > > > > > > > same > > > > > > > > set of handles as well. This should take care of all the > issues > > > we > > > > > have > > > > > > > > here. > > > > > > > > > > > > > > > > -Val > > > > > > > > > > > > > > > > On Thu, Jan 19, 2017 at 7:26 AM, Nikita Amelchev < > > > > > [email protected] > > > > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > I have some questions about single Marshaller. > > > > > > > > > It seems not easy to merge OptimizedMarshaller with > > > > > BinaryMarshaller > > > > > > > and > > > > > > > > is > > > > > > > > > there any sense in it? > > > > > > > > > When Binary object inside Externalizable serialized with > > > > optimized > > > > > it > > > > > > > > > losing all benefits. > > > > > > > > > Will OptimizedMarshaller be supported at 2.0 version? Or to > > > merge > > > > > > they > > > > > > > is > > > > > > > > > better? > > > > > > > > > What do you think about it? > > > > > > > > > > > > > > > > > > In addition, Vladimir Ozerov, I would like to hear your > > > opinion. > > > > > > > > > > > > > > > > > > 2017-01-17 23:32 GMT+03:00 Denis Magda <[email protected] > >: > > > > > > > > > > > > > > > > > > > Someone else added you to the contributors list in JIRA. > > This > > > > is > > > > > > why > > > > > > > I > > > > > > > > > > couldn’t add you for the second time. Ignite committers, > > > please > > > > > > reply > > > > > > > > on > > > > > > > > > > the dev list if you add someone to the list. > > > > > > > > > > > > > > > > > > > > Nikita, yes, this ticket is still relevant. Go ahead and > > > assign > > > > > it > > > > > > on > > > > > > > > > > yourself. > > > > > > > > > > > > > > > > > > > > Also please you may want to help with approaching 2.0 > > release > > > > and > > > > > > > take > > > > > > > > > > care of one of the sub-tasks that must be included in > 2.0: > > > > > > > > > > https://issues.apache.org/jira/browse/IGNITE-4547 < > > > > > > > > > > https://issues.apache.org/jira/browse/IGNITE-4547> > > > > > > > > > > > > > > > > > > > > — > > > > > > > > > > Denis > > > > > > > > > > > > > > > > > > > > > On Jan 15, 2017, at 9:02 PM, Nikita Amelchev < > > > > > > [email protected] > > > > > > > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > This issue was created long ago. Is still relevant? > > > > > > > > > > > > > > > > > > > > > > JIRA account: > > > > > > > > > > > Username: NSAmelchev > > > > > > > > > > > Full Name: Amelchev Nikita > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 2017-01-14 1:52 GMT+03:00 Denis Magda < > [email protected] > > >: > > > > > > > > > > > > > > > > > > > > > >> Hi Nikita, > > > > > > > > > > >> > > > > > > > > > > >> I can’t find provided account in Ignite JIRA > > > > > > > > > > >> https://issues.apache.org/jira/browse/IGNITE < > > > > > > > > > > https://issues.apache.org/ > > > > > > > > > > >> jira/browse/IGNITE> > > > > > > > > > > >> > > > > > > > > > > >> Please create an account there and share with me. > > > > > > > > > > >> > > > > > > > > > > >> This information might be useful for you as well. > > > > > > > > > > >> > > > > > > > > > > >> Subscribe to both dev and user lists: > > > > > > > > > > >> https://ignite.apache.org/ > > community/resources.html#mail- > > > > lists > > > > > > > > > > >> > > > > > > > > > > >> Get familiar with Ignite development process described > > > here: > > > > > > > > > > >> https://cwiki.apache.org/confluence/display/IGNITE/ > > > > > > > > > Development+Process > > > > > > > > > > >> > > > > > > > > > > >> Instructions on how to contribute can be found here: > > > > > > > > > > >> https://cwiki.apache.org/ > confluence/display/IGNITE/How+ > > > > > > > > to+Contribute > > > > > > > > > > >> > > > > > > > > > > >> Project setup in Intellij IDEAL > > > > > > > > > > >> https://cwiki.apache.org/confluence/display/IGNITE/ > > > > > > Project+Setup > > > > > > > > > > >> > > > > > > > > > > >> Regards, > > > > > > > > > > >> Denis > > > > > > > > > > >> > > > > > > > > > > >>> On Jan 13, 2017, at 1:37 AM, Nikita Amelchev < > > > > > > > [email protected] > > > > > > > > > > > > > > > > > > > >> wrote: > > > > > > > > > > >>> > > > > > > > > > > >>> Hello everyone. > > > > > > > > > > >>> > > > > > > > > > > >>> I'd like to take IGNITE-2894. Can you assign to me? > > > > > > > > > > >>> > > > > > > > > > > >>> Username: NSAmelchev > > > > > > > > > > >>> > > > > > > > > > > >>> -- > > > > > > > > > > >>> Best wishes, > > > > > > > > > > >>> Amelchev Nikita > > > > > > > > > > >> > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > > > > > > > Best wishes, > > > > > > > > > > > Amelchev Nikita > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > > > > > Best wishes, > > > > > > > > > Amelchev Nikita > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > > > Best wishes, > > > > > > > Amelchev Nikita > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > Best wishes, > > > > > Amelchev Nikita > > > > > > > > > > > > > > > > > > > > -- > > Best wishes, > > Amelchev Nikita > > > -- Best wishes, Amelchev Nikita
