commons-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Henri Yandell (JIRA)" <>
Subject [jira] Commented: (LANG-574) HashCodeBuilder append(Object) could be more efficient
Date Sat, 09 Jan 2010 11:21:54 GMT


Henri Yandell commented on LANG-574:

Definite improvement when using the isArray check in HashCodeBuilder. Oddly I find it improves
Object[] dramatically too, though not long[]. [My test was for String, Object[] and long[]).
Part of the Object[] is due to dropping the instanceof and using a catch-all else, but it
doesn't seem to be all of it.

Switching to the == class doesn't improve things; I suspect the compiler might have been doing
that already.

I can get improved savings for Object[] by checking clss.getComponentType().isPrimitive(),
but it comes at a similar cost to the primitive arrays.

svn ci -m "Performance improvement per Anthony Whitford in LANG-574. Check for isArray to
short-circuit the 9 instanceof checks. Improves both non-arrays and Object[] in tests"
Sending        src/main/java/org/apache/commons/lang3/builder/
Transmitting file data .
Committed revision 897419.

> HashCodeBuilder append(Object) could be more efficient
> ------------------------------------------------------
>                 Key: LANG-574
>                 URL:
>             Project: Commons Lang
>          Issue Type: Improvement
>          Components: lang.builder.*
>    Affects Versions: 2.4
>         Environment: Sun JDK 1.6.0_17
>            Reporter: Anthony Whitford
>            Priority: Minor
>             Fix For: 3.0
> See:
> If you use *reflectionHashCode*, you will eventually call *reflectionAppend(Object)*
for every field (see lines 859 to 890).  There is a long list of _instanceOf_ checks for the
various types of arrays.  My concern is that having an array field is relatively rare, yet
the code does 9 if/else checks before computing the _hashCode_ for _one_ element.  I am thinking
that it would make a lot of sense to wrap those if/else checks for the various array types
in an *isArray* check (see Class.isArray).
> If you look at the JDK source code for *Arrays.deepToString*, you will see that it checks
if the class is an array before checking the various types.  Note that since it gets the Class,
it doesn't need to execute _instanceOf_ which is likely more expensive...
> I have not done profiling, but I would not be surprised if short circuiting these checks
could improve performance by an order of magnitude.  The assumption is that getting the Class
of an Object and asking it if it is an array is cheaper than asking _instanceOf_ 9 times.

This message is automatically generated by JIRA.
You can reply to this email to add a comment to the issue online.

View raw message