harmony-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Tim Ellison <t.p.elli...@gmail.com>
Subject Re: [jira] Created: (HARMONY-6404) possible data-reordering in some hashCode-methods (e.g. String or URL)
Date Sun, 20 Dec 2009 19:42:20 GMT
On 19/Dec/2009 20:54, Nathan Beyer wrote:
> 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?

Yes, we don't code to allow for bogus VM/compiler implementations.

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

I'm guessing you have your tongue firmly in your cheek when writing that
:-)  Yes to "correct as possible" but we don't account for the never to
be implemented edge cases that are possible by the spec.

Put another way, we don't fix things that are not broken.  I challenge
you to show an implementation that would do the read re-ordering as you
suggest.

In any case, if you want to make the changes, go ahead, I don't think it
will break/fix anything.

> 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.

They are different because the multiple loads by the same thread make a
difference in the equals() method.  If the hashCode changes between
loads by the same thread then the equals would answer the wrong value.

However, the hashCode method is different because a thread can't return
the wrong value by reading the hashCode twice.  There may be a data
race, with two threads both calculating the hash, but they will simply
compute the same value and store it, so no harm done.

Regards,
Tim

> [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