ignite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Valentin Kulichenko <valentin.kuliche...@gmail.com>
Subject Re: IGNITE-3244
Date Wed, 08 Feb 2017 22:12:13 GMT
Generally, this code is correct. marshalToBinary must return array of
BinaryObject instances when object array is passes as an input. However,
the fact that we convert even first class citizen arrays (e.g. String[])
looks wrong. Perhaps we need to check that and return arrays like this
as-is, without any changes. The same is true for doReadObjectArray method -
if compType is first class citizen (e.g., String), we should create
corresponding array (e.g., String[]) instead of Object[].

-Val

On Wed, Feb 8, 2017 at 9:51 AM, Denis Magda <dmagda@apache.org> wrote:

> Hi Aleksey,
>
> >> type IDs correctly attached to class descriptors. The issue comes from
> >> BinaryUtils#doReadObjectArray() . Presently, unmarshalled array's
> elements
> >> stored in Object[], one need to write the following :
> >> Object[] arr = (Object[])Array.newInstance(compType, len);
> >> instead of
> >> Object[] arr = deserialize ? (Object[])Array.newInstance(compType, len)
>
> This is for occasions when an array needs to be returned in NON
> deserialized form meaning that its elements still will be of binary type.
> When “deserialize” flag is set to “true” the elements will be fully
> deserialized. I think that we should properly set “deserialize” or
> “keepBinary” flag somewhere.
>
> Vovan, could join the conversation and share your thoughts?
>
> >> Moreover, in CacheObjectBinaryProcessorImpl#marshalToBinary() we
> >> explicitly marshal objects array into Object[] and then write it
> further. I
> >> wonder why we dont just marshal -> unmarshal it ?
>
>
> This kind of legacy code that was there for a while. I can’t recall the
> reason why we have it but I’m sure there is a sense for that.
>
> Val, Vovan, do you remember?
>
> —
> Denis
>
> > On Feb 7, 2017, at 5:05 AM, ALEKSEY KUZNETSOV <alkuznetsov.sb@gmail.com>
> wrote:
> >
> > I have a solution on this task, this is pull request into my forked
> > repository : https://github.com/voipp/ignite/pull/6
> >
> > пн, 6 февр. 2017 г. в 16:03, ALEKSEY KUZNETSOV <alkuznetsov.sb@gmail.com
> >:
> >
> >> Moreover, in CacheObjectBinaryProcessorImpl#marshalToBinary() we
> >> explicitly marshal objects array into Object[] and then write it
> further. I
> >> wonder why we dont just marshal -> unmarshal it ?
> >>
> >> пн, 6 февр. 2017 г. в 15:58, ALEKSEY KUZNETSOV <
> alkuznetsov.sb@gmail.com>:
> >>
> >> type IDs correctly attached to class descriptors. The issue comes from
> >> BinaryUtils#doReadObjectArray() . Presently, unmarshalled array's
> elements
> >> stored in Object[], one need to write the following :
> >> Object[] arr = (Object[])Array.newInstance(compType, len);
> >> instead of
> >> Object[] arr = deserialize ? (Object[])Array.newInstance(compType, len)
> >>
> >> пн, 6 февр. 2017 г. в 11:16, Vladimir Ozerov <vozerov@gridgain.com>:
> >>
> >> Denis,
> >>
> >> We need to ensure that type IDs are attached for both arrays and
> >> collections/maps. May be we have it for arrays already, but I doubt it
> is
> >> so for generic containers.
> >>
> >> On Fri, Feb 3, 2017 at 8:59 PM, Denis Magda <dmagda@apache.org> wrote:
> >>
> >>> If a value can be serialized to BinaryObject then it will be
> serialized.
> >>> This piece of the code works perfectly well as far as I understand.
> >>>
> >>> I would agree with Vladimir that the binary protocol needs to be
> >> revisited
> >>> or the way we serialize/deserialize arrays. Presently, when an array of
> >> any
> >>> type is serialized with BinaryWriterExImpl.doWriteObjectArray we write
> >>> that this is the array
> >>>
> >>> out.unsafeWriteByte(GridBinaryMarshaller.OBJ_ARR);
> >>> and right below we store the actual type
> >>>
> >>> out.unsafeWriteInt(desc.typeId())
> >>>
> >>> In my understanding, in the layout of binary data we have all the data
> >>> needed to deserialize an array back properly.
> >>>
> >>> Vovan, is there anything we are missing?
> >>>
> >>> —
> >>> Denis
> >>>
> >>>> On Feb 3, 2017, at 7:09 AM, ALEKSEY KUZNETSOV <
> >> alkuznetsov.sb@gmail.com>
> >>> wrote:
> >>>>
> >>>> There is javax.cache.integration.CacheWriter#write() method which
> >> writes
> >>>> key and value, wrapped in CacheEntryImpl.
> >>>> In case of class arrays, values must be in form of BinaryObject or
> >>>> serialized class arrays ?
> >>>>
> >>>> чт, 2 февр. 2017 г. в 19:57, Denis Magda <dmagda@apache.org>:
> >>>>
> >>>>> Yes, this is exactly the reason why this ticket is created.
> Presently,
> >>>>> binary marshaller ignores custom type for arrays. We need to find
a
> >> way
> >>> how
> >>>>> to handle this.
> >>>>>
> >>>>> —
> >>>>> Denis
> >>>>>
> >>>>>> On Feb 2, 2017, at 6:23 AM, ALEKSEY KUZNETSOV <
> >>> alkuznetsov.sb@gmail.com>
> >>>>> wrote:
> >>>>>>
> >>>>>> I founded that cache.get(i) actually returns an array, containing
> >>>>>> TestObject. But somehow the type of returned value is Object[]
not
> >>>>>> TestObject[]
> >>>>>>
> >>>>>> ср, 1 февр. 2017 г. в 22:26, Denis Magda <dmagda@apache.org>:
> >>>>>>
> >>>>>>> Excellent, please share the way you want to fix the issue
with the
> >>>>>>> community. You might get a valuable feedback before getting
down to
> >>>>> coding.
> >>>>>>>
> >>>>>>> —
> >>>>>>> Denis
> >>>>>>>
> >>>>>>>> On Feb 1, 2017, at 1:18 AM, ALEKSEY KUZNETSOV <
> >>>>> alkuznetsov.sb@gmail.com>
> >>>>>>> wrote:
> >>>>>>>>
> >>>>>>>> will take https://issues.apache.org/jira/browse/IGNITE-3244
> >>>>>>>> --
> >>>>>>>>
> >>>>>>>> *Best Regards,*
> >>>>>>>>
> >>>>>>>> *Kuznetsov Aleksey*
> >>>>>>>
> >>>>>>> --
> >>>>>>
> >>>>>> *Best Regards,*
> >>>>>>
> >>>>>> *Kuznetsov Aleksey*
> >>>>>
> >>>>> --
> >>>>
> >>>> *Best Regards,*
> >>>>
> >>>> *Kuznetsov Aleksey*
> >>>
> >>>
> >>
> >> --
> >>
> >> *Best Regards,*
> >>
> >> *Kuznetsov Aleksey*
> >>
> >> --
> >>
> >> *Best Regards,*
> >>
> >> *Kuznetsov Aleksey*
> >>
> > --
> >
> > *Best Regards,*
> >
> > *Kuznetsov Aleksey*
>
>

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