harmony-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Nathan Beyer <ndbe...@apache.org>
Subject Re: [jira] Created: (HARMONY-6404) possible data-reordering in some hashCode-methods (e.g. String or URL)
Date Sat, 19 Dec 2009 20:54:03 GMT
On Sat, Dec 19, 2009 at 2:26 PM, Tim Ellison <t.p.ellison@gmail.com> wrote:
> On 19/Dec/2009 18:57, Nathan Beyer wrote:
>> RE: https://issues.apache.org/jira/browse/HARMONY-6404
>>
>> I posted up two proposed patches for String, please comment on which
>> is preferable. One is the change previously mentioned, the other is a
>> slightly bigger reorganization.
>
> Neither.  There is nothing wrong with the original code.
>
> Any JVM or compiler that would actually move a load up above a
> conditional store to the same variable would be seriously broken all round.

Does that really matter? Shouldn't the Java source be written such
that it's as correct as possible according to the language spec and
related specs?

Regardless, this isn't about ordering, its about 'this.hashCode' being
read multiple times -- which is the exact type of change that was just
fixed in the equals method [1]. How are these two methods different?
Why change one and not the other.

-Nathan

[1] https://issues.apache.org/jira/browse/HARMONY-6405

>
> Regards,
> Tim
>
>
>> On Sat, Dec 19, 2009 at 4:04 AM, Egor Pasko <egor.pasko@gmail.com> wrote:
>>> On the 0x68A day of Apache Harmony Tim Ellison wrote:
>>>> On 12/Dec/2009 10:54, Egor Pasko wrote:
>>>>> On the 0x685 day of Apache Harmony Nathan Beyer wrote:
>>>> <snip>
>>>>>> In any case, it does seem a pinch more efficient to only do one read
>>>>>> of hashCode ... switch up the code to be something like this.
>>>>>>
>>>>>> public int hashCode() {
>>>>>>     final int hash = hashCode;
>>>>>>     if (hash == 0) {
>>>>>>         if (count == 0) {
>>>>>>             return 0;
>>>>>>         }
>>>>>>         for (int i = offset; i < count + offset; i++) {
>>>>>>             hash = value[i] + ((hash << 5) - hash);
>>>>>>         }
>>>>>>         hashCode = hash;
>>>>> one more 'return hash' here, please :)
>>>> Why?
>>> argh, what was I thinking about? the original Nathan's code is great.
>>>
>>> --
>>> Egor Pasko
>>>
>>>
>>
>

Mime
View raw message