On Jun 17, 2013, at 10:06 AM, Aleksey Shipilev <aleksey.shipi...@oracle.com> wrote:
> On 06/16/2013 11:44 PM, Alan Bateman wrote: >> On 13/06/2013 12:47, Aleksey Shipilev wrote: >>> Friendly reminder for the reviewers. >>> >>> On 06/10/2013 07:53 PM, Aleksey Shipilev wrote: >>>> This is the follow-up on the issue Doug identified: >>>> >>>> http://mail.openjdk.java.net/pipermail/core-libs-dev/2013-June/017798.html >>>> >>>> >>>> I had reworked the patch, webrev is here: >>>> http://cr.openjdk.java.net/~shade/8016236/webrev.01/ >>> The current webrev is here: >>> http://cr.openjdk.java.net/~shade/8016236/webrev.02/ >>> >> I'm just catching up on this thread, webrev.02 looks very good to me. I >> can sponsor it. > > Thanks! I'll appreciate this. > >> One comment on ClassRepositoryHolder is that it's "yet another class" >> (we have a proliferation of holder classes since we started using this >> idiom). It makes me wonder if it might be better to move the constant to >> ClassRepository. > > Ok, makes sense. I moved it to ClassRepository itself. Given we have the > ClassRepository field in Class, the static initialization will be > performed on the first class load. I have no problems having it public, > since it is protected enough from the external modifications. > > The new webrev is here: > http://cr.openjdk.java.net/~shade/8016236/webrev.03/ > Looks good to me. Paul. > Testing: > - Linux/x86_64/release build OK > - Linux/x86_64/release passes java/lang/reflect jtreg > - microbenchmark scores are still intact > > -Aleksey.
