ignite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Nikita Amelchev <nsamelc...@gmail.com>
Subject Re: IGNITE-2894 - Binary object inside of Externalizable still serialized with OptimizedMarshaller
Date Wed, 23 Aug 2017 15:11:46 GMT
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 <
valentin.kulichenko@gmail.com>:

> 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 <nsamelchev@gmail.com>
> 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 <vozerov@gridgain.com>:
> >
> > > 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 <
> > > valentin.kulichenko@gmail.com> 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 <
> nsamelchev@gmail.com
> > >
> > > > 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 <
> > > > > valentin.kulichenko@gmail.com>:
> > > > >
> > > > > > 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 <
> > > nsamelchev@gmail.com
> > > > >
> > > > > > 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 <
> > > > > > > valentin.kulichenko@gmail.com>:
> > > > > > >
> > > > > > > > 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 <
> > > > > nsamelchev@gmail.com
> > > > > > >
> > > > > > > > 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 <dmagda@apache.org
> >:
> > > > > > > > >
> > > > > > > > > > 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 <
> > > > > > nsamelchev@gmail.com
> > > > > > > >
> > > > > > > > > > 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
<
> dmagda@apache.org
> > >:
> > > > > > > > > > >
> > > > > > > > > > >> 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 <
> > > > > > > nsamelchev@gmail.com
> > > > > > > > >
> > > > > > > > > > >> 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

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message