harmony-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Vijay Menon <...@google.com>
Subject Re: [jira] Created: (HARMONY-6404) possible data-reordering in some hashCode-methods (e.g. String or URL)
Date Sat, 09 Jan 2010 21:53:57 GMT
2010/1/9 Egor Pasko <egor.pasko@gmail.com>

> On the 0x68E day of Apache Harmony Nathan Beyer wrote:
> > On Sun, Dec 20, 2009 at 1:42 PM, Tim Ellison <t.p.ellison@gmail.com>
> wrote:
> >> 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.
> >
> > So what's all the fuss, then? According to this guy [1] ... our code
> > is broken and he helped define the memory model. This is the guy
> > suggesting the issue. Our code is almost exactly like the same he
> > describes as broken. The only difference is we have an extra check on
> > 'count'.
> >
> > At this point, I just want to understand.
> >
> > [1]
> http://jeremymanson.blogspot.com/2008/12/benign-data-races-in-java.html
>
> Sorry for delayed answer (was on vacation).
>
> After all the fuss I think Tim is right. JMM is a model that is close
> to real implementations except some corner cases like this. JMM allows
> slightly more than any JVM implementation would do. That's a good
> excersise to think about and be warned, but not a lot more.
>
> If our goal is to please all possible JVM implementations, we should
> bother, but that would be far from the main goal of the project. Until
> we have a meaningful implementation to make two memory loads in the
> place where only one is required, we should not take any action on
> hashCode.
>
>
I hate to belabor the point, but the code is legally broken and Nathan's
proposed fix is simple and straightforward.  Why not fix it?

I agree that it's unlikely to break on x86.  But, I think it can break on
practical implementations for other architectures.  From memory, StarJIT's
Itanium backend would do exactly the reordering Jeremy Manson mentions in
his blog post under certain profile information and scheduling constraints.

Vijay


> --
> Egor Pasko
>
>

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