ignite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Dmitriy Setrakyan <dsetrak...@apache.org>
Subject Re: Portables hash code.
Date Thu, 29 Oct 2015 05:52:52 GMT
On Wed, Oct 28, 2015 at 8:12 AM, Vladimir Ozerov <vozerov@gridgain.com>
wrote:

> I do not like idea of predefined alogrithms, because we will lack
> flexibility. What if user has backing Java/.Net class where hash code is
> calculated differently? What if user *cannot change* this class for some
> reason?
> Any hard-coded hash code algorithm looks like a shortcut for me.
>
> Moreover, hash code can be different on different paltforms for the same
> base type. E.g. in Java hash code of string is more or less constant, while
> in .Net it changes from release to release. Java UUID has one hash code,
> .Net counterpart Guid has another hash code. Etc.. As a result user will be
> able to use this solution without any problems only if he operates on
> primitive types.
>
> Why not abstracting out this to some interface and provide default
> implementation with proposed algorithm?
>

How about adding JdbcTypeHasher interface, which will look like this:

interface JdbcTypeHasher {
    public int hashCode(IgniteObject o, Collection<String> fields);
}

User should be optionally set this interface at the JdbcType level. If not
set, then we use our default implementation.

Will this work?



>
> On Wed, Oct 28, 2015 at 5:49 PM, Alexey Kuznetsov <akuznetsov@gridgain.com
> >
> wrote:
>
> > Igniters,
> >
> > After some thinking on hash code generation for portables and POJO store
> we
> > could do the following:
> >
> > 1) As Dmitriy suggested in [1] a boolean flag "isHashCode" to
> > JdbcTypeField. If true then this field will be used for hash code
> > calculation by pojo store.
> > 2) Add to all PortableBuilder.setField() methods one more boolean
> argument
> > isHashCode.
> > 3) PortableBuilder should store fields in order they were added.
> > 4) PortableBuilder will calculate hash code if at least one field was
> added
> > with this flag.
> >    I think we could use algorithm like this (pseudocode):
> >                 int hashCode = 0;
> >
> >                 for (field : fields) {
> >                     if (field.isHashCode)
> >                         hashCode = 31 * hashCode + (field.value != null
> > ? field.value.hashCode() : 0);
> >             }
> > .
> > 5) Describe in documentation that ORDER of fields is VERY IMPORTANT for
> > hash code calculation and equality checks.
> >
> > Thoughs?
> >
> > [1]
> >
> https://cwiki.apache.org/confluence/display/IGNITE/Jdbc+Store+Configuration
> >
> > On Wed, Oct 28, 2015 at 1:06 PM, Alexey Kuznetsov <
> akuznetsov@gridgain.com
> > >
> > wrote:
> >
> > > Denis,
> > >
> > > My use case:
> > >
> > > In my legacy database I have table Organization with compound ID:
> (sbjID;
> > > orgID).
> > > For Pojo store will be generated key with 2 fields inside:
> > > OrganizationKey(int sbjID; int orgID).
> > >
> > > And store will load this key into cache.
> > >
> > > Of course usual scenario will be when key is primitive (int or long),
> but
> > > portable also could be a key.
> > >
> > > And one more case: several types in same cache: for store we should
> have
> > > different key types.
> > >
> > >
> > > On Wed, Oct 28, 2015 at 12:58 PM, Denis Magda <dmagda@gridgain.com>
> > wrote:
> > >
> > >> Sure, makes sense to add a documentation section to PortableObject
> > >> describing default implementation of hashCode/equals.
> > >>
> > >> However, I don’t see any significant reason why the end user needs to
> > use
> > >> portable objects as keys.
> > >>
> > >> —
> > >> Denis
> > >>
> > >> > On 28 окт. 2015 г., at 7:42, Alexey Kuznetsov <
> > akuznetsov@gridgain.com>
> > >> wrote:
> > >> >
> > >> > Dmitriy,
> > >> >
> > >> >>> How about we check that dashcode is not 0 when storing portable
> keys
> > >> in
> > >> > cache?
> > >> >
> > >> > And print warning in log? Sounds good for me.
> > >> >
> > >> > -----
> > >> >
> > >> > But I will try to describe once again my concern.
> > >> >
> > >> > I'm don't know anything about portables. So, I open javadoc and see
> > some
> > >> > idiomatic code like this:
> > >> >
> > >> > builder = ...
> > >> >
> > >> > for ( fileds) {
> > >> >  builder.setField(..)
> > >> > }
> > >> >
> > >> > portable = builder.build();
> > >> >
> > >> > And I in my PojoStore I do as described.
> > >> >
> > >> > If there was a NOTE in javadoc about hashCode() it will save 4 hours
> > of
> > >> > debug for me :)
> > >> >
> > >> >
> > >> >
> > >> >
> > >> > On Wed, Oct 28, 2015 at 11:32 AM, Dmitriy Setrakyan <
> > >> dsetrakyan@apache.org>
> > >> > wrote:
> > >> >
> > >> >> How about we check that dashcode is not 0 when storing portable
> keys
> > in
> > >> >> cache?
> > >> >>
> > >> >> On Tue, Oct 27, 2015 at 8:42 PM, Alexey Kuznetsov <
> > >> akuznetsov@gridgain.com
> > >> >>>
> > >> >> wrote:
> > >> >>
> > >> >>> Andrey, thanks.
> > >> >>>
> > >> >>> Actually I use this not well documented method to solve my
> problem.
> > >> >>>
> > >> >>> And I'm proposed to at least add more info for PortableBuilder
> about
> > >> this
> > >> >>> method.
> > >> >>>
> > >> >>> On Wed, Oct 28, 2015 at 10:19 AM, Andrey Kornev <
> > >> >> andrewkornev@hotmail.com>
> > >> >>> wrote:
> > >> >>>
> > >> >>>> Alexey,
> > >> >>>>
> > >> >>>> PortableBuilder has an (un-documented) method hashCode(int
> > hashCode)
> > >> >> that
> > >> >>>> should be used to explicitly set the hashCode for the
portable
> > >> instance
> > >> >>>> being built. I'm not sure why this has been designed this
way,
> but
> > >> I'm
> > >> >>>> guessing that since the PortableBuilder is pretty dumb
and it
> > >> wouldn't
> > >> >>> know
> > >> >>>> which fields to use for hash code computation (in some
cases
> you'd
> > >> only
> > >> >>>> want to include specific portable fields rather than all
fields).
> > >> >>>>
> > >> >>>> Regards
> > >> >>>> Andrey
> > >> >>>>
> > >> >>>>> Date: Wed, 28 Oct 2015 09:49:07 +0700
> > >> >>>>> Subject: Portables hash code.
> > >> >>>>> From: akuznetsov@gridgain.com
> > >> >>>>> To: dev@ignite.apache.org
> > >> >>>>>
> > >> >>>>> Igniters,
> > >> >>>>>
> > >> >>>>> I'm working on  [1] "IGNITE-1753 Rework CacheJdbcPojoStore
to
> new
> > >> >> API."
> > >> >>>>>
> > >> >>>>> And one of subtasks is to support portable objects
with JDBC
> > store.
> > >> >>>>>
> > >> >>>>> I implemented this and during tests found a huge performance
> drop
> > >> >> when
> > >> >>> I
> > >> >>>>> have PortableObject as key.
> > >> >>>>>
> > >> >>>>> After some debugging I found that all my portable
objects have
> > >> >> hashCode
> > >> >>>> = 0.
> > >> >>>>> I'm using PortableBuilder to build my portable objects.
> > >> >>>>>
> > >> >>>>> And I expected that PortableBuilder will calculate
proper hash
> > code
> > >> >> for
> > >> >>>> me
> > >> >>>>> out of the box.
> > >> >>>>>
> > >> >>>>> I think we should at least describe in PortableBuilder
javadocs
> > that
> > >> >> by
> > >> >>>>> default PortableBuilder will return zero hashcode?
> > >> >>>>>
> > >> >>>>> Or we should calculate hashcode in PortableBuilder.build()
> method?
> > >> >>>>>
> > >> >>>>> Or may be we could add boolean argument
> > >> PortableBuilder.build(boolean
> > >> >>>>> generateHashCode)?
> > >> >>>>>
> > >> >>>>> Thoughts?
> > >> >>>>>
> > >> >>>>> P.S. After I added manual hashcode calculation to
my
> > >> >> CacheJdbcPojoStore
> > >> >>>>> performance drop is gone away.
> > >> >>>>>
> > >> >>>>>
> > >> >>>>> [1] https://issues.apache.org/jira/browse/IGNITE-1753
> > >> >>>>> --
> > >> >>>>> Alexey Kuznetsov
> > >> >>>>> GridGain Systems
> > >> >>>>> www.gridgain.com
> > >> >>>>
> > >> >>>>
> > >> >>>
> > >> >>>
> > >> >>>
> > >> >>> --
> > >> >>> Alexey Kuznetsov
> > >> >>> GridGain Systems
> > >> >>> www.gridgain.com
> > >> >>>
> > >> >>
> > >> >
> > >> >
> > >> >
> > >> > --
> > >> > Alexey Kuznetsov
> > >> > GridGain Systems
> > >> > www.gridgain.com
> > >>
> > >>
> > >
> > >
> > > --
> > > Alexey Kuznetsov
> > > GridGain Systems
> > > www.gridgain.com
> > >
> >
> >
> >
> > --
> > Alexey Kuznetsov
> > GridGain Systems
> > www.gridgain.com
> >
>

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