Hi!
First of all, when array is serialized, marshaller actually DO
PRESERVE type of element (seel
org.apache.ignite.internal.binary.BinaryUtils#doReadObjectArray and
org.apache.ignite.internal.binary.BinaryWriterExImpl#doWriteObjectArray).
AFAIK, the motivation of Nickolay proposal, is the fact, that during
call of PlatformService we do additional marshal/unmarshall step and
during this step we loose array type info
See org.apache.ignite.internal.binary.BinaryReaderExImpl#readObjectDetached
and org.apache.ignite.internal.binary.BinaryUtils#doReadObjectArray)
In order to handle this problem, we can just add some wrapper that
contains element type info and use it in our INTERNAL API.
I just don't understand why we should change IgniteBinary API.
>> It was designed as a data storage format, and the fact
that Ignite uses it elsewhere is a fundamental design flaw. BTW, this is
something we're trying to fix in 3.0.
Please, do not. There are many cases when this can really improve
performance. Reflection is always slower than low level api and many
users are happy with low level API.
сб, 1 мая 2021 г. в 00:51, Valentin Kulichenko <valentin.kuliche...@gmail.com>:
>
> Hi Nikolay,
>
> Is there a specific motivation behind your proposal? I do acknowledge that
> the semantics of the toBinary method is a little weird, but my concern is
> that the way it works with arrays is just an example. Are you suggesting
> changing collections, primitives, and other "first citizen" data types as
> well? To me, that looks like a huge risky change without a clear value.
>
> I actually believe that binary format *should not* be used as a universal
> serde mechanism. It was designed as a data storage format, and the fact
> that Ignite uses it elsewhere is a fundamental design flaw. BTW, this is
> something we're trying to fix in 3.0.
>
> That said, I'm not necessarily against the change (especially if we do not
> change the default behavior). But I would like to better understand its
> scope (e.g., arrays only or beyond?), as well as get some examples of use
> cases that would benefit from the change.
>
> -Val
>
>
> On Fri, Apr 30, 2021 at 7:35 AM Nikolay Izhikov <nizhi...@apache.org> wrote:
>
> > Hello, Ilya.
> >
> > Thanks for the feedback!
> >
> > > For me it sounds like something we would like to do in 3.0
> >
> > Ignite 3 is a very long way to go, so I prefer to target this fix in
> > Ignite 2.x.
> >
> > > I think making it default "true" is a breaking change and is not
> > possible in a minor release
> >
> > Yes, you are correct it is a breaking change.
> > It seems for me, we all agreed that breaking changes are possible in minor
> > releases.
> >
> > Anyway, if we will decide do not to enable this feature by default it’s OK
> > for me.
> > We still can implement it and improve the binary SerDe mechanism.
> >
> >
> > > 30 апр. 2021 г., в 17:23, Ilya Kasnacheev <ilya.kasnach...@gmail.com>
> > написал(а):
> > >
> > > Hello!
> > >
> > > For me it sounds like something we would like to do in 3.0 (if indeed it
> > > will have arrays as possible value (or key) type), but doing it in 2.x
> > > raises concerns whether it has enough time left to stabilize.
> > >
> > > Also, I think making it default "true" is a breaking change and is not
> > > possible in a minor release, case in point,
> > > IGNITE_BINARY_SORT_OBJECT_FIELDS is still waiting for 3.0 and it is less
> > > destructive.
> > >
> > > Of course I would also like to hear what other community members think.
> > >
> > > Regards,
> > > --
> > > Ilya Kasnacheev
> > >
> > >
> > > пт, 30 апр. 2021 г. в 17:16, Nikolay Izhikov <nizhi...@apache.org>:
> > >
> > >> Igniters,
> > >>
> > >> Want to clarify my proposal about new array store format.
> > >> I think we should store array in special binary wrapper that will keep
> > >> original component type
> > >>
> > >> ```
> > >> public class BinaryArrayWrapper implements BinaryObjectEx,
> > Externalizable {
> > >> /** Type ID. */
> > >> private int compTypeId;
> > >>
> > >> /** Raw data. */
> > >> private String compClsName;
> > >>
> > >> /** Value. */
> > >> private Object[] arr;
> > >>
> > >> // Further implementation.
> > >> }
> > >> ```
> > >>
> > >>
> > >>> 30 апр. 2021 г., в 16:31, Nikolay Izhikov <nizhikov....@gmail.com>
> > >> написал(а):
> > >>>
> > >>> Hello, Igniters.
> > >>>
> > >>> Currently, binary marshaller works as follows(Say, we have a class
> > >> `User` then):
> > >>>
> > >>> IgniteBinary#toBinary(User)` -> BinaryObject
> > >>> IgniteBinary#toBinary(User[])` -> Object[]
> > >>> IgniteBinary#toBinary(Object[])` -> Object[]
> > >>>
> > >>> This means, that we lose array component type information during binary
> > >> serialization.
> > >>> AFAIK, it’s a design choice made during binary infrastructure
> > >> development.
> > >>>
> > >>> This lead to the following disadvantages:
> > >>>
> > >>> 1. `IgniteBinary` can’t be used as a universal SerDe mechanism.
> > >>> 2. Ignite internals(service grid, .Net calls) contains many tweaks and
> > >> hacks to deal with custom user array and still has many issues [1]
> > >>>
> > >>> I propose to make breaking changes and fix the custom user array SeDe
> > as
> > >> follows:
> > >>>
> > >>> 1. Implement binary serialization that correctly Ser and Deser
> > >> array using some kind of the wrapper (BinaryArrayWrapper).
> > >>>
> > >>> IgniteBinary#toBinary(User)` -> BinaryObject
> > >>> IgniteBinary#toBinary(User[])` -> BinaryObject
> > >>> IgniteBinary#toBinary(Object[])` -> BinaryObject
> > >>>
> > >>> 2. Introduce system flag `IGNITE_USE_BINARY_ARRAY` that enables
> > >> correct SerDe of arrays. The default value is false to keep backward
> > >> compatibility in the next Ignite release(2.11).
> > >>>
> > >>> 3. Set `IGNITE_USE_BINARY_ARRAY` to `true` in the ongoing Ignite
> > >> releases (2.12).
> > >>>
> > >>> WDYT?
> > >>>
> > >>> [1] https://issues.apache.org/jira/browse/IGNITE-14299
> > >>
> > >>
> >
> >
--
Sincerely yours, Ivan Daschinskiy
