Re: custom extension for make/SourceRevision.gmk

[Christian Thalinger]

Done: https://bugs.openjdk.java.net/browse/JDK-8210008

> On Aug 23, 2018, at 4:56 PM, Erik Joelsson  wrote:
> 

Re: custom extension for make/SourceRevision.gmk

[Christian Thalinger]

> On Jul 19, 2018, at 9:17 PM, Christian Thalinger  
> wrote:
> 
> 
> 
>> On Jul 19, 2018, at 2:31 PM, Erik Joelsson > <mailto:erik.joels...@oracle.com>> wrote:
>> 
>> I can do that. Do you have a bug?
>> 
> No.

Sorry, was on vacation… I don’t see the change in the repo. Did you file one?

>> /Erik
>> 
>> On 2018-07-19 10:57, Christian Thalinger wrote:
>>> 
>>> 
>>> On Thu, Jul 19, 2018 at 1:11 PM Erik Joelsson >> <mailto:erik.joels...@oracle.com>> wrote:
>>> This looks good to me, but will need coordination when pushed as I said 
>>> earlier.
>>> 
>>> 
>>> Do you want to push it so it’s easier?
>>> 
>>> /Erik
>>> 
>>> On 2018-07-19 10:04, Christian Thalinger wrote:
>>>> 
>>>> 
>>>>> On Jul 19, 2018, at 12:57 PM, Erik Joelsson >>>> <mailto:erik.joels...@oracle.com>> wrote:
>>>>> 
>>>>> 
>>>>> 
>>>>> On 2018-07-19 09:54, Christian Thalinger wrote:
>>>>>> 
>>>>>> 
>>>>>>> On Jul 19, 2018, at 12:44 PM, Erik Joelsson >>>>>> <mailto:erik.joels...@oracle.com>> wrote:
>>>>>>> 
>>>>>>> 
>>>>>>> On 2018-07-19 09:16, Christian Thalinger wrote:
>>>>>>>> 
>>>>>>>> 
>>>>>>>> Well, the issue is this:
>>>>>>>> 
>>>>>>>> exploded-image: exploded-image-base release-file
>>>>>>>> 
>>>>>>>>   release-file: create-source-revision-tracker
>>>>>>>> 
>>>>>>>> store-source-revision:
>>>>>>>> +($(CD) $(TOPDIR)/make && $(MAKE) $(MAKE_ARGS) -f 
>>>>>>>> SourceRevision.gmk store-source-revision)
>>>>>>>> 
>>>>>>>> create-source-revision-tracker:
>>>>>>>> +($(CD) $(TOPDIR)/make && $(MAKE) $(MAKE_ARGS) -f 
>>>>>>>> SourceRevision.gmk create-source-revision-tracker)
>>>>>>>> 
>>>>>>>> We need these targets because all isn’t really used.
>>>>>>>> 
>>>>>>> Ah, the all target is tricking me and should be removed if not called 
>>>>>>> from anywhere. Then your suggested patch is good (except for missing 
>>>>>>> the :=).
>>>>>> 
>>>>>> Do you want me to remove the all: target?
>>>>>> 
>>>>> Yes, that would be a good cleanup to avoid confusion.
>>>> 
>>>> How about this:
>>>> 
>>>> diff --git a/make/SourceRevision.gmk b/make/SourceRevision.gmk
>>>> index 10dd943..6d4a706 100644
>>>> --- a/make/SourceRevision.gmk
>>>> +++ b/make/SourceRevision.gmk
>>>> @@ -1,5 +1,5 @@
>>>>  #
>>>> -# Copyright (c) 2016, Oracle and/or its affiliates. All rights reserved.
>>>> +# Copyright (c) 2016, 2018, Oracle and/or its affiliates. All rights 
>>>> reserved.
>>>>  # DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
>>>>  #
>>>>  # This code is free software; you can redistribute it and/or modify it
>>>> @@ -23,12 +23,10 @@
>>>>  # questions.
>>>>  #
>>>>  
>>>> -default: all
>>>> -
>>>>  include $(SPEC)
>>>>  include MakeBase.gmk
>>>>  
>>>> -$(eval $(call IncludeCustomExtension, SourceRevision.gmk))
>>>> +$(eval $(call IncludeCustomExtension, SourceRevision-pre.gmk))
>>>>  
>>>>  
>>>> 
>>>>  # Keep track of what source revision is used to create the build, by 
>>>> creating
>>>> @@ -94,11 +92,14 @@ ifneq ($(and $(HG), $(wildcard $(TOPDIR)/.hg)), )
>>>>  
>>>>$(eval $(call CreateSourceRevisionFile, $(STORED_SOURCE_REVISION)))
>>>>  
>>>> -  store-source-revision: $(STORED_SOURCE_REVISION)
>>>> +  hg-store-source-revision: $(STORED_SOURCE_REVISION)
>>>>  
>>>>$(eval $(call CreateSourceRevisionFile, $(SOURCE_REVISION_TRACKER)))
>>>>  
>>>> -  create-source-revision-tracker: $(SOURCE_REVISION_TRACKER)
>>>> +  hg-create-sou

Re: custom extension for make/SourceRevision.gmk

[Christian Thalinger]

> On Jul 19, 2018, at 2:31 PM, Erik Joelsson  wrote:
> 
> I can do that. Do you have a bug?
> 
No.
> /Erik
> 
> On 2018-07-19 10:57, Christian Thalinger wrote:
>> 
>> 
>> On Thu, Jul 19, 2018 at 1:11 PM Erik Joelsson > <mailto:erik.joels...@oracle.com>> wrote:
>> This looks good to me, but will need coordination when pushed as I said 
>> earlier.
>> 
>> 
>> Do you want to push it so it’s easier?
>> 
>> /Erik
>> 
>> On 2018-07-19 10:04, Christian Thalinger wrote:
>>> 
>>> 
>>>> On Jul 19, 2018, at 12:57 PM, Erik Joelsson >>> <mailto:erik.joels...@oracle.com>> wrote:
>>>> 
>>>> 
>>>> 
>>>> On 2018-07-19 09:54, Christian Thalinger wrote:
>>>>> 
>>>>> 
>>>>>> On Jul 19, 2018, at 12:44 PM, Erik Joelsson >>>>> <mailto:erik.joels...@oracle.com>> wrote:
>>>>>> 
>>>>>> 
>>>>>> On 2018-07-19 09:16, Christian Thalinger wrote:
>>>>>>> 
>>>>>>> 
>>>>>>> Well, the issue is this:
>>>>>>> 
>>>>>>> exploded-image: exploded-image-base release-file
>>>>>>> 
>>>>>>>   release-file: create-source-revision-tracker
>>>>>>> 
>>>>>>> store-source-revision:
>>>>>>> +($(CD) $(TOPDIR)/make && $(MAKE) $(MAKE_ARGS) -f 
>>>>>>> SourceRevision.gmk store-source-revision)
>>>>>>> 
>>>>>>> create-source-revision-tracker:
>>>>>>> +($(CD) $(TOPDIR)/make && $(MAKE) $(MAKE_ARGS) -f 
>>>>>>> SourceRevision.gmk create-source-revision-tracker)
>>>>>>> 
>>>>>>> We need these targets because all isn’t really used.
>>>>>>> 
>>>>>> Ah, the all target is tricking me and should be removed if not called 
>>>>>> from anywhere. Then your suggested patch is good (except for missing the 
>>>>>> :=).
>>>>> 
>>>>> Do you want me to remove the all: target?
>>>>> 
>>>> Yes, that would be a good cleanup to avoid confusion.
>>> 
>>> How about this:
>>> 
>>> diff --git a/make/SourceRevision.gmk b/make/SourceRevision.gmk
>>> index 10dd943..6d4a706 100644
>>> --- a/make/SourceRevision.gmk
>>> +++ b/make/SourceRevision.gmk
>>> @@ -1,5 +1,5 @@
>>>  #
>>> -# Copyright (c) 2016, Oracle and/or its affiliates. All rights reserved.
>>> +# Copyright (c) 2016, 2018, Oracle and/or its affiliates. All rights 
>>> reserved.
>>>  # DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
>>>  #
>>>  # This code is free software; you can redistribute it and/or modify it
>>> @@ -23,12 +23,10 @@
>>>  # questions.
>>>  #
>>>  
>>> -default: all
>>> -
>>>  include $(SPEC)
>>>  include MakeBase.gmk
>>>  
>>> -$(eval $(call IncludeCustomExtension, SourceRevision.gmk))
>>> +$(eval $(call IncludeCustomExtension, SourceRevision-pre.gmk))
>>>  
>>>  
>>> 
>>>  # Keep track of what source revision is used to create the build, by 
>>> creating
>>> @@ -94,11 +92,14 @@ ifneq ($(and $(HG), $(wildcard $(TOPDIR)/.hg)), )
>>>  
>>>$(eval $(call CreateSourceRevisionFile, $(STORED_SOURCE_REVISION)))
>>>  
>>> -  store-source-revision: $(STORED_SOURCE_REVISION)
>>> +  hg-store-source-revision: $(STORED_SOURCE_REVISION)
>>>  
>>>$(eval $(call CreateSourceRevisionFile, $(SOURCE_REVISION_TRACKER)))
>>>  
>>> -  create-source-revision-tracker: $(SOURCE_REVISION_TRACKER)
>>> +  hg-create-source-revision-tracker: $(SOURCE_REVISION_TRACKER)
>>> +
>>> +  STORE_SOURCE_REVISION_TARGET := hg-store-source-revision
>>> +  CREATE_SOURCE_REVISION_TRACKER_TARGET := 
>>> hg-create-source-revision-tracker
>>>  
>>>  else
>>># Not using HG
>>> @@ -106,28 +107,39 @@ else
>>>ifneq ($(wildcard $(STORED_SOURCE_REVISION)), )
>>>  # We have a stored source revision (.src-rev)
>>>  
>>> -store-source-revision:
>>> +src-store-source-revision:
>>> $(call LogInf

Re: custom extension for make/SourceRevision.gmk

[Christian Thalinger]

On Thu, Jul 19, 2018 at 1:11 PM Erik Joelsson 
wrote:

> This looks good to me, but will need coordination when pushed as I said
> earlier.
>

Do you want to push it so it’s easier?

/Erik
>
> On 2018-07-19 10:04, Christian Thalinger wrote:
>
>
>
> On Jul 19, 2018, at 12:57 PM, Erik Joelsson 
> wrote:
>
>
>
> On 2018-07-19 09:54, Christian Thalinger wrote:
>
>
>
> On Jul 19, 2018, at 12:44 PM, Erik Joelsson 
> wrote:
>
>
> On 2018-07-19 09:16, Christian Thalinger wrote:
>
>
>
> Well, the issue is this:
>
> exploded-image: exploded-image-base release-file
>
>   release-file: create-source-revision-tracker
>
> store-source-revision:
> +($(CD) $(TOPDIR)/make && $(MAKE) $(MAKE_ARGS) -f
> SourceRevision.gmk store-source-revision)
>
> create-source-revision-tracker:
> +($(CD) $(TOPDIR)/make && $(MAKE) $(MAKE_ARGS) -f
> SourceRevision.gmk create-source-revision-tracker)
>
> We need these targets because all isn’t really used.
>
> Ah, the all target is tricking me and should be removed if not called from
> anywhere. Then your suggested patch is good (except for missing the :=).
>
>
> Do you want me to remove the all: target?
>
> Yes, that would be a good cleanup to avoid confusion.
>
>
> How about this:
>
> diff --git a/make/SourceRevision.gmk b/make/SourceRevision.gmk
> index 10dd943..6d4a706 100644
> --- a/make/SourceRevision.gmk
> +++ b/make/SourceRevision.gmk
> @@ -1,5 +1,5 @@
>  #
> -# Copyright (c) 2016, Oracle and/or its affiliates. All rights reserved.
> +# Copyright (c) 2016, 2018, Oracle and/or its affiliates. All rights
> reserved.
>  # DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
>  #
>  # This code is free software; you can redistribute it and/or modify it
> @@ -23,12 +23,10 @@
>  # questions.
>  #
>
>
> -default: all
> -
>  include $(SPEC)
>  include MakeBase.gmk
>
>
> -$(eval $(call IncludeCustomExtension, SourceRevision.gmk))
> +$(eval $(call IncludeCustomExtension, SourceRevision-pre.gmk))
>
>
>
>  
> 
>  # Keep track of what source revision is used to create the build, by
> creating
> @@ -94,11 +92,14 @@ ifneq ($(and $(HG), $(wildcard $(TOPDIR)/.hg)), )
>
>
>$(eval $(call CreateSourceRevisionFile, $(STORED_SOURCE_REVISION)))
>
>
> -  store-source-revision: $(STORED_SOURCE_REVISION)
> +  hg-store-source-revision: $(STORED_SOURCE_REVISION)
>
>
>$(eval $(call CreateSourceRevisionFile, $(SOURCE_REVISION_TRACKER)))
>
>
> -  create-source-revision-tracker: $(SOURCE_REVISION_TRACKER)
> +  hg-create-source-revision-tracker: $(SOURCE_REVISION_TRACKER)
> +
> +  STORE_SOURCE_REVISION_TARGET := hg-store-source-revision
> +  CREATE_SOURCE_REVISION_TRACKER_TARGET :=
> hg-create-source-revision-tracker
>
>
>  else
># Not using HG
> @@ -106,28 +107,39 @@ else
>ifneq ($(wildcard $(STORED_SOURCE_REVISION)), )
>  # We have a stored source revision (.src-rev)
>
>
> -store-source-revision:
> +src-store-source-revision:
> $(call LogInfo, No mercurial configuration present$(COMMA) not
> updating .src-rev)
>
>
>  $(SOURCE_REVISION_TRACKER): $(STORED_SOURCE_REVISION)
> $(install-file)
>
>
> -create-source-revision-tracker: $(SOURCE_REVISION_TRACKER)
> +src-create-source-revision-tracker: $(SOURCE_REVISION_TRACKER)
>else
>  # We don't have a stored source revision. Can't do anything, really.
>
>
> -store-source-revision:
> +src-store-source-revision:
> $(call LogWarn, Error: No mercurial configuration present$(COMMA)
> cannot create .src-rev)
> exit 2
>
>
> -create-source-revision-tracker:
> +src-create-source-revision-tracker:
> $(call LogWarn, Warning: No mercurial configuration present and no
> .src-rev)
>endif
>
>
> +  STORE_SOURCE_REVISION_TARGET := src-store-source-revision
> +  CREATE_SOURCE_REVISION_TRACKER_TARGET :=
> src-create-source-revision-tracker
> +
>  endif
>
>
> -all: store-source-revision create-source-revision-tracker
>
> +
> +
> +$(eval $(call IncludeCustomExtension, SourceRevision-post.gmk))
> +
>
> +
> +
> +store-source-revision: $(STORE_SOURCE_REVISION_TARGET)
> +
> +create-source-revision-tracker: $(CREATE_SOURCE_REVISION_TRACKER_TARGET)
>
>
>  FRC: # Force target
>
>
> -.PHONY: all store-source-revision create-source-revision-tracker
> +.PHONY: store-source-revision create-source-revision-tracker
>
>
>

Re: custom extension for make/SourceRevision.gmk

[Christian Thalinger]

> On Jul 19, 2018, at 12:57 PM, Erik Joelsson  wrote:
> 
> 
> 
> On 2018-07-19 09:54, Christian Thalinger wrote:
>> 
>> 
>>> On Jul 19, 2018, at 12:44 PM, Erik Joelsson >> <mailto:erik.joels...@oracle.com>> wrote:
>>> 
>>> 
>>> On 2018-07-19 09:16, Christian Thalinger wrote:
>>>> 
>>>> 
>>>> Well, the issue is this:
>>>> 
>>>> exploded-image: exploded-image-base release-file
>>>> 
>>>>   release-file: create-source-revision-tracker
>>>> 
>>>> store-source-revision:
>>>> +($(CD) $(TOPDIR)/make && $(MAKE) $(MAKE_ARGS) -f 
>>>> SourceRevision.gmk store-source-revision)
>>>> 
>>>> create-source-revision-tracker:
>>>> +($(CD) $(TOPDIR)/make && $(MAKE) $(MAKE_ARGS) -f 
>>>> SourceRevision.gmk create-source-revision-tracker)
>>>> 
>>>> We need these targets because all isn’t really used.
>>>> 
>>> Ah, the all target is tricking me and should be removed if not called from 
>>> anywhere. Then your suggested patch is good (except for missing the :=).
>> 
>> Do you want me to remove the all: target?
>> 
> Yes, that would be a good cleanup to avoid confusion.

How about this:

diff --git a/make/SourceRevision.gmk b/make/SourceRevision.gmk
index 10dd943..6d4a706 100644
--- a/make/SourceRevision.gmk
+++ b/make/SourceRevision.gmk
@@ -1,5 +1,5 @@
 #
-# Copyright (c) 2016, Oracle and/or its affiliates. All rights reserved.
+# Copyright (c) 2016, 2018, Oracle and/or its affiliates. All rights reserved.
 # DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
 #
 # This code is free software; you can redistribute it and/or modify it
@@ -23,12 +23,10 @@
 # questions.
 #
 
-default: all
-
 include $(SPEC)
 include MakeBase.gmk
 
-$(eval $(call IncludeCustomExtension, SourceRevision.gmk))
+$(eval $(call IncludeCustomExtension, SourceRevision-pre.gmk))
 
 

 # Keep track of what source revision is used to create the build, by creating
@@ -94,11 +92,14 @@ ifneq ($(and $(HG), $(wildcard $(TOPDIR)/.hg)), )
 
   $(eval $(call CreateSourceRevisionFile, $(STORED_SOURCE_REVISION)))
 
-  store-source-revision: $(STORED_SOURCE_REVISION)
+  hg-store-source-revision: $(STORED_SOURCE_REVISION)
 
   $(eval $(call CreateSourceRevisionFile, $(SOURCE_REVISION_TRACKER)))
 
-  create-source-revision-tracker: $(SOURCE_REVISION_TRACKER)
+  hg-create-source-revision-tracker: $(SOURCE_REVISION_TRACKER)
+
+  STORE_SOURCE_REVISION_TARGET := hg-store-source-revision
+  CREATE_SOURCE_REVISION_TRACKER_TARGET := hg-create-source-revision-tracker
 
 else
   # Not using HG
@@ -106,28 +107,39 @@ else
   ifneq ($(wildcard $(STORED_SOURCE_REVISION)), )
 # We have a stored source revision (.src-rev)
 
-store-source-revision:
+src-store-source-revision:
$(call LogInfo, No mercurial configuration present$(COMMA) not updating 
.src-rev)
 
 $(SOURCE_REVISION_TRACKER): $(STORED_SOURCE_REVISION)
$(install-file)
 
-create-source-revision-tracker: $(SOURCE_REVISION_TRACKER)
+src-create-source-revision-tracker: $(SOURCE_REVISION_TRACKER)
   else
 # We don't have a stored source revision. Can't do anything, really.
 
-store-source-revision:
+src-store-source-revision:
$(call LogWarn, Error: No mercurial configuration present$(COMMA) 
cannot create .src-rev)
exit 2
 
-create-source-revision-tracker:
+src-create-source-revision-tracker:
$(call LogWarn, Warning: No mercurial configuration present and no 
.src-rev)
   endif
 
+  STORE_SOURCE_REVISION_TARGET := src-store-source-revision
+  CREATE_SOURCE_REVISION_TRACKER_TARGET := src-create-source-revision-tracker
+
 endif
 
-all: store-source-revision create-source-revision-tracker
+
+
+$(eval $(call IncludeCustomExtension, SourceRevision-post.gmk))
+
+
+
+store-source-revision: $(STORE_SOURCE_REVISION_TARGET)
+
+create-source-revision-tracker: $(CREATE_SOURCE_REVISION_TRACKER_TARGET)
 
 FRC: # Force target
 
-.PHONY: all store-source-revision create-source-revision-tracker
+.PHONY: store-source-revision create-source-revision-tracker

Re: custom extension for make/SourceRevision.gmk

[Christian Thalinger]

> On Jul 19, 2018, at 12:44 PM, Erik Joelsson  wrote:
> 
> 
> On 2018-07-19 09:16, Christian Thalinger wrote:
>> 
>> 
>> Well, the issue is this:
>> 
>> exploded-image: exploded-image-base release-file
>> 
>>   release-file: create-source-revision-tracker
>> 
>> store-source-revision:
>> +($(CD) $(TOPDIR)/make && $(MAKE) $(MAKE_ARGS) -f SourceRevision.gmk 
>> store-source-revision)
>> 
>> create-source-revision-tracker:
>> +($(CD) $(TOPDIR)/make && $(MAKE) $(MAKE_ARGS) -f SourceRevision.gmk 
>> create-source-revision-tracker)
>> 
>> We need these targets because all isn’t really used.
>> 
> Ah, the all target is tricking me and should be removed if not called from 
> anywhere. Then your suggested patch is good (except for missing the :=).

Do you want me to remove the all: target?

> 
> /Erik
>>>> +store-source-revision: $(STORE_SOURCE_REVISION_TARGET)
>>>> +
>>>> +create-source-revision-tracker: $(CREATE_SOURCE_REVISION_TRACKER_TARGET)
>>>> +
>>>>  all: store-source-revision create-source-revision-tracker
>>>>  
>>>>  FRC: # Force target
>>>> 
>>> Do you really need the separate variables? Since both of them are built by 
>>> all anyway, I would just have one variable TARGETS to which you add 
>>> everything you wish to build and finish with "all: $(TARGETS)".
>>> 
>>> /Erik
>> 
> 

Re: custom extension for make/SourceRevision.gmk

[Christian Thalinger]

> On Jul 19, 2018, at 12:46 PM, Erik Joelsson  wrote:
> 
> One thing though. Since you are renaming the top file, we need to coordinate 
> pushing of this change since we are relying on the old name.

We can keep the old name, if you want, but that wouldn’t be very consistent 
with other files.

> 
> /Erik
> 
> 
> On 2018-07-19 09:44, Erik Joelsson wrote:
>> 
>> On 2018-07-19 09:16, Christian Thalinger wrote:
>>> 
>>> 
>>> Well, the issue is this:
>>> 
>>> exploded-image: exploded-image-base release-file
>>> 
>>>   release-file: create-source-revision-tracker
>>> 
>>> store-source-revision:
>>>   +($(CD) $(TOPDIR)/make && $(MAKE) $(MAKE_ARGS) -f SourceRevision.gmk 
>>> store-source-revision)
>>> 
>>> create-source-revision-tracker:
>>>   +($(CD) $(TOPDIR)/make && $(MAKE) $(MAKE_ARGS) -f SourceRevision.gmk 
>>> create-source-revision-tracker)
>>> 
>>> We need these targets because all isn’t really used.
>>> 
>> Ah, the all target is tricking me and should be removed if not called from 
>> anywhere. Then your suggested patch is good (except for missing the :=).
>> 
>> /Erik
>>>>> +store-source-revision: $(STORE_SOURCE_REVISION_TARGET)
>>>>> +
>>>>> +create-source-revision-tracker: $(CREATE_SOURCE_REVISION_TRACKER_TARGET)
>>>>> +
>>>>>  all: store-source-revision create-source-revision-tracker
>>>>> 
>>>>>  FRC: # Force target
>>>>> 
>>>> Do you really need the separate variables? Since both of them are built by 
>>>> all anyway, I would just have one variable TARGETS to which you add 
>>>> everything you wish to build and finish with "all: $(TARGETS)".
>>>> 
>>>> /Erik
>>> 
>> 
> 

Re: custom extension for make/SourceRevision.gmk

[Christian Thalinger]

> On Jul 19, 2018, at 12:08 PM, Erik Joelsson  wrote:
> 
> Hello,
> On 2018-07-19 07:43, Christian Thalinger wrote:
>> 
>> 
>>> On Jul 18, 2018, at 3:28 PM, Christian Thalinger >> <mailto:cthalin...@twitter.com>> wrote:
>>> 
>>> 
>>> 
>>>> On Jul 18, 2018, at 1:46 PM, Erik Joelsson >>> <mailto:erik.joels...@oracle.com>> wrote:
>>>> 
>>>> Hello Christian,
>>>> 
>>>> Sometimes we need hooks both close to the beginning and close to the end 
>>>> of the file, and in that case we create a SourceBundle-post.gmk. The 
>>>> recommended position of the post inclusion is right before the typical 
>>>> "all: $(TARGETS)" declaration. This file has the all target depend 
>>>> explicitly on a list of phony targets and no TARGETS variable, so I would 
>>>> recommend changing that to building a TARGETS variable like we usually do. 
>>>> That way you can create a SourceBundle-post.gmk and clear the TARGETS 
>>>> variable from any targets you don't want to run from the open file. Does 
>>>> that sound ok?
>>> 
>>> Yes, that would be great.  In JDK 11, please :-)
>> 
>> Ok, this is the only way I could make it work:
>> 
>> diff --git a/make/SourceRevision.gmk b/make/SourceRevision.gmk
>> index 10dd943..13ea407 100644
>> --- a/make/SourceRevision.gmk
>> +++ b/make/SourceRevision.gmk
>> @@ -28,7 +28,7 @@ default: all
>>  include $(SPEC)
>>  include MakeBase.gmk
>>  
>> -$(eval $(call IncludeCustomExtension, SourceRevision.gmk))
>> +$(eval $(call IncludeCustomExtension, SourceRevision-pre.gmk))
>>  
>>  
>> 
>>  # Keep track of what source revision is used to create the build, by 
>> creating
>> @@ -94,11 +94,14 @@ ifneq ($(and $(HG), $(wildcard $(TOPDIR)/.hg)), )
>>  
>>$(eval $(call CreateSourceRevisionFile, $(STORED_SOURCE_REVISION)))
>>  
>> -  store-source-revision: $(STORED_SOURCE_REVISION)
>> +  hg-store-source-revision: $(STORED_SOURCE_REVISION)
>>  
>>$(eval $(call CreateSourceRevisionFile, $(SOURCE_REVISION_TRACKER)))
>>  
>> -  create-source-revision-tracker: $(SOURCE_REVISION_TRACKER)
>> +  hg-create-source-revision-tracker: $(SOURCE_REVISION_TRACKER)
>> +
> These assignments should be using :=. Applies further down as well.
>> +  STORE_SOURCE_REVISION_TARGET = hg-store-source-revision
>> +  CREATE_SOURCE_REVISION_TRACKER_TARGET = hg-create-source-revision-tracker
>>  
>>  else
>># Not using HG
>> @@ -106,26 +109,39 @@ else
>>ifneq ($(wildcard $(STORED_SOURCE_REVISION)), )
>>  # We have a stored source revision (.src-rev)
>>  
>> -store-source-revision:
>> +src-store-source-revision:
>> $(call LogInfo, No mercurial configuration present$(COMMA) not 
>> updating .src-rev)
>>  
>>  $(SOURCE_REVISION_TRACKER): $(STORED_SOURCE_REVISION)
>> $(install-file)
>>  
>> -create-source-revision-tracker: $(SOURCE_REVISION_TRACKER)
>> +src-create-source-revision-tracker: $(SOURCE_REVISION_TRACKER)
>>else
>>  # We don't have a stored source revision. Can't do anything, really.
>>  
>> -store-source-revision:
>> +src-store-source-revision:
>> $(call LogWarn, Error: No mercurial configuration present$(COMMA) 
>> cannot create .src-rev)
>> exit 2
>>  
>> -create-source-revision-tracker:
>> +src-create-source-revision-tracker:
>> $(call LogWarn, Warning: No mercurial configuration present and no 
>> .src-rev)
>>endif
>>  
>> +  STORE_SOURCE_REVISION_TARGET = src-store-source-revision
>> +  CREATE_SOURCE_REVISION_TRACKER_TARGET = src-create-source-revision-tracker
>> +
>>  endif
>>  
>> +
>> +
>> +$(eval $(call IncludeCustomExtension, SourceRevision-post.gmk))
>> +
>> +
>> +
> I would suggest using the variables directly on the all: line instead of 
> declaring more phony targets. 

Well, the issue is this:

exploded-image: exploded-image-base release-file

  release-file: create-source-revision-tracker

store-source-revision:
+($(CD) $(TOPDIR)/make && $(MAKE) $(MAKE_ARGS) -f SourceRevision.gmk 
store-source-revision)

create-source-revision-tracker:
+($(CD) $(TOPDIR)/make && $(MAKE) $(MAKE_ARGS) -f SourceRevision.gmk 
create-source-revision-tracker)

We need these targets because all isn’t really used.

>> +store-source-revision: $(STORE_SOURCE_REVISION_TARGET)
>> +
>> +create-source-revision-tracker: $(CREATE_SOURCE_REVISION_TRACKER_TARGET)
>> +
>>  all: store-source-revision create-source-revision-tracker
>>  
>>  FRC: # Force target
>> 
> Do you really need the separate variables? Since both of them are built by 
> all anyway, I would just have one variable TARGETS to which you add 
> everything you wish to build and finish with "all: $(TARGETS)".
> 
> /Erik

Re: custom extension for make/SourceRevision.gmk

[Christian Thalinger]

> On Jul 18, 2018, at 3:28 PM, Christian Thalinger  
> wrote:
> 
> 
> 
>> On Jul 18, 2018, at 1:46 PM, Erik Joelsson  wrote:
>> 
>> Hello Christian,
>> 
>> Sometimes we need hooks both close to the beginning and close to the end of 
>> the file, and in that case we create a SourceBundle-post.gmk. The 
>> recommended position of the post inclusion is right before the typical "all: 
>> $(TARGETS)" declaration. This file has the all target depend explicitly on a 
>> list of phony targets and no TARGETS variable, so I would recommend changing 
>> that to building a TARGETS variable like we usually do. That way you can 
>> create a SourceBundle-post.gmk and clear the TARGETS variable from any 
>> targets you don't want to run from the open file. Does that sound ok?
> 
> Yes, that would be great.  In JDK 11, please :-)

Ok, this is the only way I could make it work:

diff --git a/make/SourceRevision.gmk b/make/SourceRevision.gmk
index 10dd943..13ea407 100644
--- a/make/SourceRevision.gmk
+++ b/make/SourceRevision.gmk
@@ -28,7 +28,7 @@ default: all
 include $(SPEC)
 include MakeBase.gmk
 
-$(eval $(call IncludeCustomExtension, SourceRevision.gmk))
+$(eval $(call IncludeCustomExtension, SourceRevision-pre.gmk))
 
 

 # Keep track of what source revision is used to create the build, by creating
@@ -94,11 +94,14 @@ ifneq ($(and $(HG), $(wildcard $(TOPDIR)/.hg)), )
 
   $(eval $(call CreateSourceRevisionFile, $(STORED_SOURCE_REVISION)))
 
-  store-source-revision: $(STORED_SOURCE_REVISION)
+  hg-store-source-revision: $(STORED_SOURCE_REVISION)
 
   $(eval $(call CreateSourceRevisionFile, $(SOURCE_REVISION_TRACKER)))
 
-  create-source-revision-tracker: $(SOURCE_REVISION_TRACKER)
+  hg-create-source-revision-tracker: $(SOURCE_REVISION_TRACKER)
+
+  STORE_SOURCE_REVISION_TARGET = hg-store-source-revision
+  CREATE_SOURCE_REVISION_TRACKER_TARGET = hg-create-source-revision-tracker
 
 else
   # Not using HG
@@ -106,26 +109,39 @@ else
   ifneq ($(wildcard $(STORED_SOURCE_REVISION)), )
 # We have a stored source revision (.src-rev)
 
-store-source-revision:
+src-store-source-revision:
$(call LogInfo, No mercurial configuration present$(COMMA) not updating 
.src-rev)
 
 $(SOURCE_REVISION_TRACKER): $(STORED_SOURCE_REVISION)
$(install-file)
 
-create-source-revision-tracker: $(SOURCE_REVISION_TRACKER)
+src-create-source-revision-tracker: $(SOURCE_REVISION_TRACKER)
   else
 # We don't have a stored source revision. Can't do anything, really.
 
-store-source-revision:
+src-store-source-revision:
$(call LogWarn, Error: No mercurial configuration present$(COMMA) 
cannot create .src-rev)
exit 2
 
-create-source-revision-tracker:
+src-create-source-revision-tracker:
$(call LogWarn, Warning: No mercurial configuration present and no 
.src-rev)
   endif
 
+  STORE_SOURCE_REVISION_TARGET = src-store-source-revision
+  CREATE_SOURCE_REVISION_TRACKER_TARGET = src-create-source-revision-tracker
+
 endif
 
+
+
+$(eval $(call IncludeCustomExtension, SourceRevision-post.gmk))
+
+
+
+store-source-revision: $(STORE_SOURCE_REVISION_TARGET)
+
+create-source-revision-tracker: $(CREATE_SOURCE_REVISION_TRACKER_TARGET)
+
 all: store-source-revision create-source-revision-tracker
 
 FRC: # Force target

Re: custom extension for make/SourceRevision.gmk

[Christian Thalinger]

> On Jul 18, 2018, at 1:46 PM, Erik Joelsson  wrote:
> 
> Hello Christian,
> 
> Sometimes we need hooks both close to the beginning and close to the end of 
> the file, and in that case we create a SourceBundle-post.gmk. The recommended 
> position of the post inclusion is right before the typical "all: $(TARGETS)" 
> declaration. This file has the all target depend explicitly on a list of 
> phony targets and no TARGETS variable, so I would recommend changing that to 
> building a TARGETS variable like we usually do. That way you can create a 
> SourceBundle-post.gmk and clear the TARGETS variable from any targets you 
> don't want to run from the open file. Does that sound ok?

Yes, that would be great.  In JDK 11, please :-)

> 
> /Erik
> 
> 
> On 2018-07-18 10:31, Christian Thalinger wrote:
>> Here at Twitter our OpenJDK clone lives in a GIT repository and we would 
>> like to use the source-revision feature of the release file.
>> 
>> I have all the custom extension logic in place to create the revision 
>> tracker:
>> 
>> $ cat 
>> build/macosx-x86_64-normal-server-release/support/src-rev/source-revision-tracker
>> .:ea60d3b1efc0
>> 
>> but the way make/SourceRevision.gmk is structured (the custom extensions is 
>> included at the top of the file) always triggers the warning:
>> 
>> $ make
>> Building target 'default (exploded-image)' in configuration 
>> 'macosx-x86_64-normal-server-release'
>> Warning: No mercurial configuration present and no .src-rev
>> Finished building target 'default (exploded-image)' in configuration 
>> 'macosx-x86_64-normal-server-release'
>> 
>> Is there a way so silence the warning without restructuring the upstream 
>> file?  And if no, can we change it so it works?
> 

custom extension for make/SourceRevision.gmk

[Christian Thalinger]

Here at Twitter our OpenJDK clone lives in a GIT repository and we would like 
to use the source-revision feature of the release file.

I have all the custom extension logic in place to create the revision tracker:

$ cat 
build/macosx-x86_64-normal-server-release/support/src-rev/source-revision-tracker
 
.:ea60d3b1efc0

but the way make/SourceRevision.gmk is structured (the custom extensions is 
included at the top of the file) always triggers the warning:

$ make
Building target 'default (exploded-image)' in configuration 
'macosx-x86_64-normal-server-release'
Warning: No mercurial configuration present and no .src-rev
Finished building target 'default (exploded-image)' in configuration 
'macosx-x86_64-normal-server-release'

Is there a way so silence the warning without restructuring the upstream file?  
And if no, can we change it so it works?

Re: [10] RFR(S) 8186462: [Graal] build Graal regardless AOT build

[Christian Thalinger]

> On Aug 28, 2017, at 10:18 AM, Vladimir Kozlov  
> wrote:
> 
> http://cr.openjdk.java.net/~kvn/8186462/webrev/
> https://bugs.openjdk.java.net/browse/JDK-8186462
> 
> We are planning to have Graal as experimental JIT compiler on linux-x64 in 
> next release. For that we should enable its build independent from AOT build.

Needless to say I am very happy to see this.  The change looks good.

> 
> Thanks,
> Vladimir

Re: RFR: JDK-8186115 - libelf still referenced after 8172670

[Christian Thalinger]

> On Aug 15, 2017, at 8:55 PM, Bob Vandette  wrote:
> 
> Please review these changes for JDK 10 that remove all dependencies on libelf 
> from the JDK
> build system.
> 
> Bug:
> https://bugs.openjdk.java.net/browse/JDK-8186115 
> 
> 
> Webrev:
> http://cr.openjdk.java.net/~bobv/8186115/ 
> 
Looks good.

> 
> Compiler team:  Please check CodeSectionProcessor.  Is the stepping through 
> all
> foreign call still necessary?  I only removed the comment.
> 
> Thanks,
> Bob.
> 
>  

Re: [10] RFR: 8182487: Add Unsafe.objectFieldOffset(Class, String)

[Christian Thalinger]

> On Jun 20, 2017, at 3:14 AM, Claes Redestad  wrote:
> 
> Hi,
> 
> as a startup optimization, we can avoid a number of reflective operations on
> variouscore classes by adding a specialized objectFieldOffset method taking
> Class and String rather than Field:
> 
> Webrev: https://bugs.openjdk.java.net/browse/JDK-8182487
> Hotspot: http://cr.openjdk.java.net/~redestad/8182487/hotspot.00 
> 

While you are cleaning this up, could we remove that code:

+static inline void throw_new(JNIEnv *env, const char *ename) {
+  char buf[100];
+
+  jio_snprintf(buf, 100, "%s%s", "java/lang/", ename);

and instead pass the whole java/lang/* string?

> JDK: http://cr.openjdk.java.net/~redestad/8182487/jdk.00
> 
> On startup tests this reduces executed instructions by ~1-2M, depending on
> how many of the touched classes are loaded.
> 
> Since all uses of this would throw an Error, InternalError or
> ExceptionInInitializerError if the field was missing - effectively
> aborting VM execution - it felt reasonable to simplify the code to
> consistently throw InternalError and remove a number of distracting
> try-catch blocks.
> 
> Thanks!
> 
> /Claes

Re: RFR: 8177845: Need a mechanism to load Graal

[Christian Thalinger]

> On Apr 27, 2017, at 11:51 AM, Doug Simon <doug.si...@oracle.com> wrote:
> 
>> 
>> On 27 Apr 2017, at 18:44, Christian Thalinger <cthalin...@twitter.com> wrote:
>> 
>> Thanks for doing this.  I have a hard time following what’s happening :-)
>> 
>> src/jdk.internal.vm.compiler/.mx.graal/suite.py
>> 
>> +  "jdklibraries" : {
>> +"JVMCI_SERVICES" : {
>> +  "path" : "lib/jvmci-services.jar",
>> +  "sourcePath" : "lib/jvmci-services.src.zip",
>> +  "optional" : False,
>> +  "jdkStandardizedSince" : "9",
>> +  "module" : "jdk.internal.vm.ci"
>> +},
>> +"JVMCI_API" : {
>> +  "path" : "lib/jvmci/jvmci-api.jar",
>> +  "sourcePath" : "lib/jvmci/jvmci-api.src.zip",
>> +  "dependencies" : [
>> +"JVMCI_SERVICES",
>> +  ],
>> +  "optional" : False,
>> +  "jdkStandardizedSince" : "9",
>> +  "module" : "jdk.internal.vm.ci"
>> +},
>> +"JVMCI_HOTSPOT" : {
>> +  "path" : "lib/jvmci/jvmci-hotspot.jar",
>> +  "sourcePath" : "lib/jvmci/jvmci-hotspot.src.zip",
>> +  "dependencies" : [
>> +"JVMCI_API",
>> +  ],
>> +  "optional" : False,
>> +  "jdkStandardizedSince" : "9",
>> +  "module" : "jdk.internal.vm.ci"
>> +},
>> +  },
>> 
>> Can you explain to me why we need this?  I don’t think these files are built 
>> in 9.
> 
> This simply allows `mx eclipseinit` to work when wanting to edit the JDK 
> Graal sources in Eclipse. As you state, this is completely by-passed by the 
> JDK make system.

Got it.

> 
>> 
>> src/jdk.internal.vm.ci/share/classes/module-info.java
>> 
>> Why can we remove the exports to jdk.internal.vm.compiler?  Because of this 
>> in JVMCIServiceLocator:
>> 
>> +ReflectionAccessJDK.openJVMCITo(getClass());
> 
> Yes. And the --add-exports in CompileJavaModules.gmk[1] and 
> Launcher-jdk.aot.gmk[2]. It also means these VM options are required in 
> upstream Graal when running the Graal junit tests.

Ok, thanks.

> 
> -Doug
> 
> [1] 
> http://cr.openjdk.java.net/~dnsimon/8177845.02/make/make/CompileJavaModules.gmk.udiff.html
>  
> <http://cr.openjdk.java.net/~dnsimon/8177845.02/make/make/CompileJavaModules.gmk.udiff.html>
> [2] 
> http://cr.openjdk.java.net/~dnsimon/8177845.02/jdk/make/launcher/Launcher-jdk.aot.gmk.udiff.html
>  
> <http://cr.openjdk.java.net/~dnsimon/8177845.02/jdk/make/launcher/Launcher-jdk.aot.gmk.udiff.html>
> 
>> 
>> ?
>> 
>>> On Apr 27, 2017, at 7:47 AM, Doug Simon <doug.si...@oracle.com> wrote:
>>> 
>>>> 
>>>> On 21 Apr 2017, at 13:46, Doug Simon <doug.si...@oracle.com> wrote:
>>>> 
>>>> There has been some discussion about whether we want to update Graal in 
>>>> the JDK at this late stage. The main (only?) risk is a regression in the 
>>>> AOT tool.
>>>> 
>>>> If we don't update Graal from upstream, then the qualified exports from 
>>>> JVMCI to jdk.internal.vm.compiler cannot be removed in JDK 9. Note that in 
>>>> addition to updating Graal to remove the qualified exports, there would 
>>>> also need to be changes in the relevant make files to add --add-exports 
>>>> options when compiling Graal and jaotc as they use the dynamically 
>>>> exported JVMCI packages.
>>>> 
>>>> I have an updated hotspot patch that adapts Graal to the JVMCI API changes:
>>>> 
>>>> http://cr.openjdk.java.net/~dnsimon/8177845/hotspot.02/
>>>> 
>>>> Note that this patch does not include the changes removing use of JDK 
>>>> internal API from Graal. Cherry picking those upstream Graal changes would 
>>>> be more work than simply doing a complete update from upstream Graal.
>>>> 
>>>> As I see it, there are 2 options here:
>>>> 
>>>> 1. Go with the current webrev (including hotspot.02 patch) and live with 
>>>> the qualified exports.
>>>> 2. Go with the current webrev (including hotspot.02 patch) and create a 
>>>> follow up bug to update Graal from upstream, perform the relevant make 
>>&

Re: RFR: 8177845: Need a mechanism to load Graal

[Christian Thalinger]

voke0(Native 
>>> Method)
>>>     at 
>>> java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
>>> at 
>>> java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
>>> at java.base/java.lang.reflect.Method.invoke(Method.java:563)
>>> at 
>>> com.sun.javatest.regtest.agent.MainWrapper$MainThread.run(MainWrapper.java:115)
>>> at java.base/java.lang.Thread.run(Thread.java:844)
>>> Caused by: java.lang.ClassCastException: 
>>> java.base/java.util.ImmutableCollections$MapN cannot be cast to 
>>> java.base/java.util.Properties
>>> at 
>>> jdk.internal.vm.compiler/org.graalvm.compiler.hotspot.HotSpotGraalCompilerFactory.getSavedProperties(HotSpotGraalCompilerFactory.java:215)
>>> ... 26 more
>>> 
>>> -Doug
>>> 
>>>> On 19 Apr 2017, at 23:26, Doug Simon <doug.si...@oracle.com> wrote:
>>>> 
>>>> I've updated http://cr.openjdk.java.net/~dnsimon/8177845/hotspot/ with 
>>>> these changes:
>>>> 
>>>> 1. JVMCIServiceLocator.getProvider(Class) is now protected
>>>> 2. JVMCIServiceLocator.getProviders(Class) now checks JVMCIPermission
>>>> 3. Rename: jdk.vm.ci.services.internal.JDK9 -> 
>>>> jdk.vm.ci.services.internal.ReflectionAccessJDK
>>>> 
>>>> -Doug
>>>> 
>>>>> On 19 Apr 2017, at 23:12, Doug Simon <doug.si...@oracle.com> wrote:
>>>>> 
>>>>>> 
>>>>>> On 19 Apr 2017, at 21:40, Christian Thalinger <cthalin...@twitter.com> 
>>>>>> wrote:
>>>>>> 
>>>>>>> 
>>>>>>> On Apr 19, 2017, at 9:27 AM, Doug Simon <doug.si...@oracle.com> wrote:
>>>>>>> 
>>>>>>>> 
>>>>>>>> On 19 Apr 2017, at 21:04, Mandy Chung <mandy.ch...@oracle.com> wrote:
>>>>>>>> 
>>>>>>>> 
>>>>>>>>> On Apr 19, 2017, at 11:55 AM, Christian Thalinger 
>>>>>>>>> <cthalin...@twitter.com> wrote:
>>>>>>>>> 
>>>>>>>>> 
>>>>>>>>>> On Apr 19, 2017, at 8:38 AM, Mandy Chung <mandy.ch...@oracle.com> 
>>>>>>>>>> wrote:
>>>>>>>>>> 
>>>>>>>>>> Since jdk.internal.vm.compiler becomes an upgradeable module, it is 
>>>>>>>>>> not hashed with java.base to allow it to be upgraded and there is no 
>>>>>>>>>> integrity check.  Such qualified export will be granted to any 
>>>>>>>>>> module named jdk.internal.vm.compiler at runtime.  The goal is for 
>>>>>>>>>> upgradeable modules not to use any internal APIs and eliminate the 
>>>>>>>>>> qualified exports.
>>>>>>>>>> 
>>>>>>>>>> The main thing is that jdk.vm.ci.services API would need to be 
>>>>>>>>>> guarded if it’s used by non-Graal modules.
>>>>>>>>> 
>>>>>>>>> This all makes sense but where is the restriction that only 
>>>>>>>>> jdk.internal.vm.compiler can use jdk.vm.ci.services?  
>>>>>>>> 
>>>>>>>> It's unqualified and no restriction in this change.
>>>>>>> 
>>>>>>> The public methods currently in jdk.vm.ci.services are:
>>>>>>> 
>>>>>>> 1. JVMCIServiceLocator.getProvider(Class)
>>>>>>> 2. JVMCIServiceLocator.getProviders(Class)
>>>>>>> 3. Services.initializeJVMCI()
>>>>>>> 4. Services.getSavedProperties()
>>>>>>> 5. Services.exportJVMCITo(Class)
>>>>>>> 6. Services.load(Class)
>>>>>>> 7. Services.loadSingle(Class, boolean)
>>>>>>> 
>>>>>>> 1 should be made protected. I'll update the webrev with this change.
>>>>>> 
>>>>>> Good.
>>>>>> 
>>>>>>> 
>>>>>>> 2 should check for JVMCIPermission. I'll update the webrev with this 
>>>>>>> change.
>>>>>> 
>>>>>> Good.
>>>>>> 
>>>>>>> 
>>>>>>> 3 is harmless from a security perspective in my opinion.
>>>>>> 
>>>>>> Would be good if one of Oracle’s security engineers could take a quick 
>>>>>> look just to be sure.
>>>>> 
>>>>> Vladimir, can you please bring this to the attention of the relevant 
>>>>> engineer.
>>>>> 
>>>>>>> 
>>>>>>> 4 checks for JVMCIPermission.
>>>>>> 
>>>>>> Ok.
>>>>>> 
>>>>>>> 
>>>>>>> 5, 6 and 7 will be removed in a follow bug that updates Graal from 
>>>>>>> upstream (and removes its usage of these methods).
>>>>>> 
>>>>>> About this, will this Graal update happen for JDK 9?
>>>>> 
>>>>> Yes.
>>>>> 
>>>>>> It’s awfully late in the cycle...
>>>>> 
>>>>> These are jigsaw related changes and I've been told jigsaw has an FC 
>>>>> exception (although I don't exactly know what that is).
>>>>> 
>>>>> -Doug

Re: RFR: 8177845: Need a mechanism to load Graal

[Christian Thalinger]

> On Apr 19, 2017, at 9:27 AM, Doug Simon <doug.si...@oracle.com> wrote:
> 
>> 
>> On 19 Apr 2017, at 21:04, Mandy Chung <mandy.ch...@oracle.com> wrote:
>> 
>> 
>>> On Apr 19, 2017, at 11:55 AM, Christian Thalinger <cthalin...@twitter.com> 
>>> wrote:
>>> 
>>> 
>>>> On Apr 19, 2017, at 8:38 AM, Mandy Chung <mandy.ch...@oracle.com> wrote:
>>>> 
>>>> Since jdk.internal.vm.compiler becomes an upgradeable module, it is not 
>>>> hashed with java.base to allow it to be upgraded and there is no integrity 
>>>> check.  Such qualified export will be granted to any module named 
>>>> jdk.internal.vm.compiler at runtime.  The goal is for upgradeable modules 
>>>> not to use any internal APIs and eliminate the qualified exports.
>>>> 
>>>> The main thing is that jdk.vm.ci.services API would need to be guarded if 
>>>> it’s used by non-Graal modules.
>>> 
>>> This all makes sense but where is the restriction that only 
>>> jdk.internal.vm.compiler can use jdk.vm.ci.services?  
>> 
>> It's unqualified and no restriction in this change.
> 
> The public methods currently in jdk.vm.ci.services are:
> 
> 1. JVMCIServiceLocator.getProvider(Class)
> 2. JVMCIServiceLocator.getProviders(Class)
> 3. Services.initializeJVMCI()
> 4. Services.getSavedProperties()
> 5. Services.exportJVMCITo(Class)
> 6. Services.load(Class)
> 7. Services.loadSingle(Class, boolean)
> 
> 1 should be made protected. I'll update the webrev with this change.

Good.

> 
> 2 should check for JVMCIPermission. I'll update the webrev with this change.

Good.

> 
> 3 is harmless from a security perspective in my opinion.

Would be good if one of Oracle’s security engineers could take a quick look 
just to be sure.

> 
> 4 checks for JVMCIPermission.

Ok.

> 
> 5, 6 and 7 will be removed in a follow bug that updates Graal from upstream 
> (and removes its usage of these methods).

About this, will this Graal update happen for JDK 9?  It’s awfully late in the 
cycle...

> 
> Does this address the security concerns?

Mostly, yes.  Thanks.

> 
> -Doug

Re: RFR: 8177845: Need a mechanism to load Graal

[Christian Thalinger]

> On Apr 19, 2017, at 8:38 AM, Mandy Chung <mandy.ch...@oracle.com> wrote:
> 
> Since jdk.internal.vm.compiler becomes an upgradeable module, it is not 
> hashed with java.base to allow it to be upgraded and there is no integrity 
> check.  Such qualified export will be granted to any module named 
> jdk.internal.vm.compiler at runtime.  The goal is for upgradeable modules not 
> to use any internal APIs and eliminate the qualified exports.
> 
> The main thing is that jdk.vm.ci.services API would need to be guarded if 
> it’s used by non-Graal modules.

This all makes sense but where is the restriction that only 
jdk.internal.vm.compiler can use jdk.vm.ci.services?  After all, this is a 
security issue.

> 
> Mandy
> 
>> On Apr 19, 2017, at 11:24 AM, Christian Thalinger <cthalin...@twitter.com> 
>> wrote:
>> 
>> I lost track a bit but why are we exporting jdk.vm.ci.services to the world 
>> now?
>> 
>> module jdk.internal.vm.ci {
>> -exports jdk.vm.ci.services to jdk.internal.vm.compiler;
>> +exports jdk.vm.ci.services;
>> 
>>> On Apr 18, 2017, at 9:16 PM, Doug Simon <doug.si...@oracle.com> wrote:
>>> 
>>> (adding hotspot-compiler-dev)
>>> 
>>> Please review these changes that make jdk.internal.vm.compiler an 
>>> upgradable compiler.
>>> The primary requirement for this is to remove all usage of JDK internals 
>>> from Graal.
>>> While this most involves changes in Graal, there remain a few capabilities 
>>> that must
>>> be exposed via JVMCI. Namely:
>>> 
>>> 1. Graal needs a copy of jdk.internal.misc.VM.savedProps so that it can 
>>> Graal initialize
>>> can use system properties that are guaranteed not to have been modified by 
>>> application
>>> code (Graal initialization is lazy and may occur after application code has 
>>> been run).
>>> 
>>> 2. Truffle needs to be able to trigger JVMCI initialization so that it can 
>>> select the Graal
>>> optimized implementation of the Truffle runtime.
>>> 
>>> These capabilities have been added to jdk.vm.ci.services.Services. A 
>>> comment has
>>> also been added to this class to denote the current methods to be removed
>>> in a subsequent bug to update the JDK copy of Graal with the upstream 
>>> version which
>>> no longer uses the methods. The methods destined for removal are:
>>> 
>>> exportJVMCITo(Class requestor)
>>> load(Class service)
>>> loadSingle(Class service, boolean required)
>>> 
>>> The first one is no longer needed as JVMCI exports itself to Graal service 
>>> providers
>>> it loads via private API and Graal re-exports[1] JVMCI to any module the 
>>> extends Graal.
>>> The latter 2 are no longer required as Graal uses the standard 
>>> ServiceLoader. This API
>>> is only useful for JVMCI-8 where a hidden JVMCI class loader is used.
>>> 
>>> These changes have been tested against upstream Graal. To make the Graal 
>>> tests pass, 2 extra
>>> minor changes were required:
>>> 
>>> 1. In JDK-8177673, HotSpotMemoryAccessProviderImpl::checkRead was added and 
>>> throws IllegalArgumentException
>>> on all error paths except one which throws AssertionError. The latter was a 
>>> mistake and has been
>>> fixed to also throw IllegalArgumentException.
>>> 2. An invalid assertion has been removed from 
>>> HotSpotResolvedObjectTypeImpl::isDefinitelyResolvedWithRespectTo.
>>> Up to JDK 8, it was true that all classes in java.* packages were loaded by 
>>> the boot class loader.
>>> This is no longer true in JDK 9 with classes like java.sql being loaded by 
>>> platform class loader.
>>> 
>>> As hinted above, a subsequent bug will be required to pull in the latest 
>>> upstream Graal and
>>> remove use of JDK internal API from the jdk.aot module. The qualified 
>>> exports to
>>> jdk.vm.internal.compiler and jdk.aot can then be removed.
>>> 
>>> -Doug
>>> 
>>> https://bugs.openjdk.java.net/browse/JDK-8177845
>>> http://cr.openjdk.java.net/~dnsimon/8177845/
>>> 
>>> [1] 
>>> http://openjdk.java.net/projects/jigsaw/spec/issues/#IndirectQualifiedReflectiveAccess
>>> 
>>> 
>>> 
>> 
> 

Re: RFR: 8177845: Need a mechanism to load Graal

[Christian Thalinger]

I lost track a bit but why are we exporting jdk.vm.ci.services to the world now?

 module jdk.internal.vm.ci {
-exports jdk.vm.ci.services to jdk.internal.vm.compiler;
+exports jdk.vm.ci.services;

> On Apr 18, 2017, at 9:16 PM, Doug Simon  wrote:
> 
> (adding hotspot-compiler-dev)
> 
> Please review these changes that make jdk.internal.vm.compiler an upgradable 
> compiler.
> The primary requirement for this is to remove all usage of JDK internals from 
> Graal.
> While this most involves changes in Graal, there remain a few capabilities 
> that must
> be exposed via JVMCI. Namely:
> 
> 1. Graal needs a copy of jdk.internal.misc.VM.savedProps so that it can Graal 
> initialize
>  can use system properties that are guaranteed not to have been modified by 
> application
>  code (Graal initialization is lazy and may occur after application code has 
> been run).
> 
> 2. Truffle needs to be able to trigger JVMCI initialization so that it can 
> select the Graal
>  optimized implementation of the Truffle runtime.
> 
> These capabilities have been added to jdk.vm.ci.services.Services. A comment 
> has
> also been added to this class to denote the current methods to be removed
> in a subsequent bug to update the JDK copy of Graal with the upstream version 
> which
> no longer uses the methods. The methods destined for removal are:
> 
> exportJVMCITo(Class requestor)
> load(Class service)
> loadSingle(Class service, boolean required)
> 
> The first one is no longer needed as JVMCI exports itself to Graal service 
> providers
> it loads via private API and Graal re-exports[1] JVMCI to any module the 
> extends Graal.
> The latter 2 are no longer required as Graal uses the standard ServiceLoader. 
> This API
> is only useful for JVMCI-8 where a hidden JVMCI class loader is used.
> 
> These changes have been tested against upstream Graal. To make the Graal 
> tests pass, 2 extra
> minor changes were required:
> 
> 1. In JDK-8177673, HotSpotMemoryAccessProviderImpl::checkRead was added and 
> throws IllegalArgumentException
>  on all error paths except one which throws AssertionError. The latter was a 
> mistake and has been
>  fixed to also throw IllegalArgumentException.
> 2. An invalid assertion has been removed from 
> HotSpotResolvedObjectTypeImpl::isDefinitelyResolvedWithRespectTo.
>  Up to JDK 8, it was true that all classes in java.* packages were loaded by 
> the boot class loader.
>  This is no longer true in JDK 9 with classes like java.sql being loaded by 
> platform class loader.
> 
> As hinted above, a subsequent bug will be required to pull in the latest 
> upstream Graal and
> remove use of JDK internal API from the jdk.aot module. The qualified exports 
> to
> jdk.vm.internal.compiler and jdk.aot can then be removed.
> 
> -Doug
> 
> https://bugs.openjdk.java.net/browse/JDK-8177845
> http://cr.openjdk.java.net/~dnsimon/8177845/
> 
> [1] 
> http://openjdk.java.net/projects/jigsaw/spec/issues/#IndirectQualifiedReflectiveAccess
> 
> 
> 

Re: What to pass to --with-custom-make-dir?

[Christian Thalinger]

> On Mar 6, 2017, at 10:17 PM, Erik Joelsson <erik.joels...@oracle.com> wrote:
> 
> 
> 
> On 2017-03-07 03:22, David Holmes wrote:
>> On 7/03/2017 6:32 AM, Christian Thalinger wrote:
>>> 
>>>> On Mar 6, 2017, at 8:51 AM, Christian Thalinger <cthalin...@twitter.com> 
>>>> wrote:
>>>> 
>>>> 
>>>>> On Mar 3, 2017, at 4:09 PM, David Holmes <david.hol...@oracle.com> wrote:
>>>>> 
>>>>> Hi Christian,
>>>>> 
>>>>> I think you need to pass an absolute directory, in which all the custom 
>>>>> files, regardless of repo, are located. That is essentially how we use it 
>>>>> - jdk/make/closed has files included from other repos. Of course that 
>>>>> only works if names are unique.
>>>> 
>>>> Absolute directory to where?  Is 
>>>> $(CUSTOM_MAKE_DIR)/lib/ServiceabilityLibraries.gmk not under 
>>>> jdk/make/closed?
>>> 
>>> After reading your message a couple of times :-) I got it. Basically pass 
>>> /repo/make/closed and have all files there.
>> 
>> It has to be an absolute directory somewhere, not relative to whatever the 
>> current make command is executing. So it could be /repo/jdk/make/closed if 
>> you wanted it to be.
>> 
>>> But why?  That doesn’t make any sense since there is a jdk/make/closed 
>>> directory.
>> 
>> IIRC this was initially a mechanism for customizing jdk/make files, then 
>> another part of the forest also wanted a "closed" custom file and so it was 
>> "enhanced" to allow that. In 9 of course this is all handled completely 
>> differently now.
>> 
> Since we didn't have a top level closed repository in JDK8, we put all common 
> closed makefiles (and autoconf) in jdk/make/closed. This is why the custom 
> makefile includes all point to the same dir in 8u.

Ahh!  That explains the behavior in 8.  I thought there was a closed repository 
in top.  Thanks.

> 
> In JDK 9 we added a top level closed repository and created a macro to handle 
> the includes so that any closed implementor may define that macro as they 
> like and consequently, put their closed additions in any structure they like.

Yes, much better.

> 
> /Erik
>> David
>> 
>>>> 
>>>>> 
>>>>> David
>>>>> 
>>>>> On 4/03/2017 9:11 AM, Rob McKenna wrote:
>>>>>> Hi Christian,
>>>>>> 
>>>>>> I'm cc'ing build-dev (and bcc'ing jdk8u-dev) as that may be a more 
>>>>>> appropriate
>>>>>> venue for this discussion.
>>>>>> 
>>>>>>  -Rob
>>>>>> 
>>>>>> On 03/03/17 10:19, Christian Thalinger wrote:
>>>>>>> At Twitter we are using the custom extension mechanism to separate our 
>>>>>>> additional code from upstream in order to minimize conflicts.  
>>>>>>> Yesterday I wanted to add a custom extension for:
>>>>>>> 
>>>>>>> jdk/make/lib/ServiceabilityLibraries.gmk
>>>>>>> 
>>>>>>> which has this include directive:
>>>>>>> 
>>>>>>> # Include custom extensions if available.
>>>>>>> -include $(CUSTOM_MAKE_DIR)/lib/ServiceabilityLibraries.gmk
>>>>>>> 
>>>>>>> We are already using the mechanism for top-level make files, e.g. 
>>>>>>> make/Main.gmk:
>>>>>>> 
>>>>>>> # Include the corresponding custom file, if present.
>>>>>>> -include $(CUSTOM_MAKE_DIR)/Main.gmk
>>>>>>> 
>>>>>>> and we a configuring with:
>>>>>>> 
>>>>>>> --with-custom-make-dir=make/closed
>>>>>>> 
>>>>>>> This works fine for make/ but not for jdk/make/:
>>>>>>> 
>>>>>>> $ make jdk CUSTOM_MAKE_DIR=make/closed
>>>>>>> …
>>>>>>> 
>>>>>>> ## Starting jdk
>>>>>>> lib/ServiceabilityLibraries.gmk:27: 
>>>>>>> make/closed/lib/ServiceabilityLibraries.gmk: No such file or directory
>>>>>>> make[2]: *** No rule to make target 
>>>>>>> `make/closed/lib/ServiceabilityLibraries.gmk'.  Stop.
>>>>>>> make[1]: *** [libs-only] Error 2
>>>>>>> make: *** [jdk-only] Error 2
>>>>>>> 
>>>>>>> (I changed "-include" to “include” to provoke the error.)
>>>>>>> 
>>>>>>> jdk/make/ files expect CUSTOM_MAKE_DIR to be just “closed” but that 
>>>>>>> doesn’t work for top-level:
>>>>>>> 
>>>>>>> $ make jdk CUSTOM_MAKE_DIR=closed
>>>>>>> /Users/cthalinger/twitter8//make/Main.gmk:35: closed/Main.gmk: No such 
>>>>>>> file or directory
>>>>>>> make: *** No rule to make target `closed/Main.gmk'. Stop.
>>>>>>> 
>>>>>>> How is this supposed to work?

Re: What to pass to --with-custom-make-dir?

[Christian Thalinger]

> On Mar 6, 2017, at 8:51 AM, Christian Thalinger <cthalin...@twitter.com> 
> wrote:
> 
> 
>> On Mar 3, 2017, at 4:09 PM, David Holmes <david.hol...@oracle.com> wrote:
>> 
>> Hi Christian,
>> 
>> I think you need to pass an absolute directory, in which all the custom 
>> files, regardless of repo, are located. That is essentially how we use it - 
>> jdk/make/closed has files included from other repos. Of course that only 
>> works if names are unique.
> 
> Absolute directory to where?  Is 
> $(CUSTOM_MAKE_DIR)/lib/ServiceabilityLibraries.gmk not under jdk/make/closed?

After reading your message a couple of times :-) I got it.  Basically pass 
/repo/make/closed and have all files there.

But why?  That doesn’t make any sense since there is a jdk/make/closed 
directory.

> 
>> 
>> David
>> 
>> On 4/03/2017 9:11 AM, Rob McKenna wrote:
>>> Hi Christian,
>>> 
>>> I'm cc'ing build-dev (and bcc'ing jdk8u-dev) as that may be a more 
>>> appropriate
>>> venue for this discussion.
>>> 
>>>   -Rob
>>> 
>>> On 03/03/17 10:19, Christian Thalinger wrote:
>>>> At Twitter we are using the custom extension mechanism to separate our 
>>>> additional code from upstream in order to minimize conflicts.  Yesterday I 
>>>> wanted to add a custom extension for:
>>>> 
>>>> jdk/make/lib/ServiceabilityLibraries.gmk
>>>> 
>>>> which has this include directive:
>>>> 
>>>> # Include custom extensions if available.
>>>> -include $(CUSTOM_MAKE_DIR)/lib/ServiceabilityLibraries.gmk
>>>> 
>>>> We are already using the mechanism for top-level make files, e.g. 
>>>> make/Main.gmk:
>>>> 
>>>> # Include the corresponding custom file, if present.
>>>> -include $(CUSTOM_MAKE_DIR)/Main.gmk
>>>> 
>>>> and we a configuring with:
>>>> 
>>>> --with-custom-make-dir=make/closed
>>>> 
>>>> This works fine for make/ but not for jdk/make/:
>>>> 
>>>> $ make jdk CUSTOM_MAKE_DIR=make/closed
>>>> …
>>>> 
>>>> ## Starting jdk
>>>> lib/ServiceabilityLibraries.gmk:27: 
>>>> make/closed/lib/ServiceabilityLibraries.gmk: No such file or directory
>>>> make[2]: *** No rule to make target 
>>>> `make/closed/lib/ServiceabilityLibraries.gmk'.  Stop.
>>>> make[1]: *** [libs-only] Error 2
>>>> make: *** [jdk-only] Error 2
>>>> 
>>>> (I changed "-include" to “include” to provoke the error.)
>>>> 
>>>> jdk/make/ files expect CUSTOM_MAKE_DIR to be just “closed” but that 
>>>> doesn’t work for top-level:
>>>> 
>>>> $ make jdk CUSTOM_MAKE_DIR=closed
>>>> /Users/cthalinger/twitter8//make/Main.gmk:35: closed/Main.gmk: No such 
>>>> file or directory
>>>> make: *** No rule to make target `closed/Main.gmk'.  Stop.
>>>> 
>>>> How is this supposed to work?
> 

Re: What to pass to --with-custom-make-dir?

[Christian Thalinger]

> On Mar 3, 2017, at 4:09 PM, David Holmes <david.hol...@oracle.com> wrote:
> 
> Hi Christian,
> 
> I think you need to pass an absolute directory, in which all the custom 
> files, regardless of repo, are located. That is essentially how we use it - 
> jdk/make/closed has files included from other repos. Of course that only 
> works if names are unique.

Absolute directory to where?  Is 
$(CUSTOM_MAKE_DIR)/lib/ServiceabilityLibraries.gmk not under jdk/make/closed?

> 
> David
> 
> On 4/03/2017 9:11 AM, Rob McKenna wrote:
>> Hi Christian,
>> 
>> I'm cc'ing build-dev (and bcc'ing jdk8u-dev) as that may be a more 
>> appropriate
>> venue for this discussion.
>> 
>>-Rob
>> 
>> On 03/03/17 10:19, Christian Thalinger wrote:
>>> At Twitter we are using the custom extension mechanism to separate our 
>>> additional code from upstream in order to minimize conflicts.  Yesterday I 
>>> wanted to add a custom extension for:
>>> 
>>> jdk/make/lib/ServiceabilityLibraries.gmk
>>> 
>>> which has this include directive:
>>> 
>>> # Include custom extensions if available.
>>> -include $(CUSTOM_MAKE_DIR)/lib/ServiceabilityLibraries.gmk
>>> 
>>> We are already using the mechanism for top-level make files, e.g. 
>>> make/Main.gmk:
>>> 
>>> # Include the corresponding custom file, if present.
>>> -include $(CUSTOM_MAKE_DIR)/Main.gmk
>>> 
>>> and we a configuring with:
>>> 
>>> --with-custom-make-dir=make/closed
>>> 
>>> This works fine for make/ but not for jdk/make/:
>>> 
>>> $ make jdk CUSTOM_MAKE_DIR=make/closed
>>> …
>>> 
>>> ## Starting jdk
>>> lib/ServiceabilityLibraries.gmk:27: 
>>> make/closed/lib/ServiceabilityLibraries.gmk: No such file or directory
>>> make[2]: *** No rule to make target 
>>> `make/closed/lib/ServiceabilityLibraries.gmk'.  Stop.
>>> make[1]: *** [libs-only] Error 2
>>> make: *** [jdk-only] Error 2
>>> 
>>> (I changed "-include" to “include” to provoke the error.)
>>> 
>>> jdk/make/ files expect CUSTOM_MAKE_DIR to be just “closed” but that doesn’t 
>>> work for top-level:
>>> 
>>> $ make jdk CUSTOM_MAKE_DIR=closed
>>> /Users/cthalinger/twitter8//make/Main.gmk:35: closed/Main.gmk: No such file 
>>> or directory
>>> make: *** No rule to make target `closed/Main.gmk'.  Stop.
>>> 
>>> How is this supposed to work?

Re: Running jaotc with an external Graal

[Christian Thalinger]

> On Feb 14, 2017, at 4:38 AM, Andrew Haley  wrote:
> 
> On 14/02/17 14:37, Alan Bateman wrote:
>> --patch-module can be used to patch any module in the boot layer. So if 
>> you are looking to override or add classes then --patch-module should work.
> 
> Aha!  Thanks,

Does it?

Re: RFR: JDK-8172670: AOT Platform Support for Windows and Mac OS X x64 (Linux and Solaris too)

[Christian Thalinger]

This is amazing!  Awesome work.  I’m glad this got done so soon.

> On Feb 8, 2017, at 9:29 AM, Bob Vandette  wrote:
> 
> SUMMARY:
> 
> Please review the following set of changes that adds Ahead-of-time 
> compilation support for Mac OSX and 
> Windows x64 in JDK 10.  Linux and Solaris x64 AOT support has also been 
> updated to be consistent with 
> the new 100% Java based binary container support included in this changeset.
> 
> This change also removes the build and runtime dependency on the external 
> libelf library and header files.
> 
> Once this change is integrated Ahead of time support will be available for 
> the following set of Platforms:
> 
> - Linux x64
> - Windows x64
> - MacOSX x64
> - Solaris x64
> 
> RFE:
> 
> https://bugs.openjdk.java.net/browse/JDK-8172670 
> 
> 
> WEBREVS:
> 
> TOP LEVEL
> —
> 
> http://cr.openjdk.java.net/~bobv/8172670/webrev.01/
> 
> JDK
> ——
> 
> http://cr.openjdk.java.net/~bobv/8172670/jdk/webrev.01/
> 
> HOTSPOT
> —
> 
> Full Hotspot Webrev:
> http://cr.openjdk.java.net/~bobv/8172670/hotspot/webrev.01/ 
> 
> 
> 
> If you’d prefer to review things in smaller chunks, here are the hotspot 
> changes for Linux and
> Solaris including the libelf removal:
> http://cr.openjdk.java.net/~bobv/8172670/hotspot/webrev-linux.01/ 
> 
> 
> Files added to support Mac OSX:
> http://cr.openjdk.java.net/~bobv/8172670/hotspot/webrev-mac.01/ 
> 
> 
> Files added to provide Windows support:
> http://cr.openjdk.java.net/~bobv/8172670/hotspot/webrev-win.01/ 
> 
> 
> Changes to the jtreg tests for AOT:
> http://cr.openjdk.java.net/~bobv/8172670/hotspot/webrev-test.01/ 
> 
> 
> 
> TESTING:
> 
> 1. JPRT has been run and passes with these changes
> 2. jtreg AOT tests pass on all supported platforms
> 3. AOT compiling of java.base completes and can run basic Java programs on 
> all supported platforms
> 
> NOTE:
> 
> 1. Although these test run correctly on Windows, jtreg AOT tests have been 
> temporarily disabled for this platform.
> This is due to an internal JPRT system configuration issue which will 
> hopefully get resolved soon.  AOT requires 
> access to a local toolchain and our JPRT systems do not regularly install 
> Visual Studio.
> 
> 
> Thanks,
> Bob.

Re: RFR: 8145337: [JVMCI] JVMCI initialization with SecurityManager installed fails: java.security.AccessControlException: access denied

[Christian Thalinger]

> On Jan 26, 2017, at 7:40 AM, Doug Simon  wrote:
> 
>> 
>> On 26 Jan 2017, at 17:55, Mandy Chung  wrote:
>> 
>> 
>>> On Jan 26, 2017, at 3:13 AM, Doug Simon  wrote:
>>> 
 
 jdk.vm.compiler is defined by the application class loader and it’s used 
 by AOT tool.  I wonder why it has to run with security manager.
>>> 
>>> Without java.security.AllPermission, the policy for jdk.vm.compiler 
>>> required to get through a bootstrap (i.e., java -server 
>>> -XX:+UnlockExperimentalVMOptions -Djava.security.manager 
>>> -XX:+BootstrapJVMCI -XX:+UseJVMCICompiler -version) is show below 
>>> (annotated with comments denoting the methods requiring the permissions):
>>> 
>>> :
>> 
>> Are -XX:+BootstrapJVMCI -XX:+UseJVMCICompiler supported to use at runtime?
> 
> Not sure I understand your question - they cannot be used at any other time 
> apart from runtime.

The question is if these command line options are supported by Oracle in JDK 9. 
 The answer used to be no but that might have changed.  Someone from Oracle 
needs to chime in.

Having said that, it would be a shame if we don’t make jdk.vm.compiler a 
trusted system component because it obviously is.

We have to do it at some point anyway so why not now…

> 
>>> There’s no guarantee that this is all the permissions required since not 
>>> all code paths are exercised during bootstrap.
>>> 
 You can reference JDK tools such as jdk.compiler and jdk.jlink that are 
 not granted with any permission.
>>> 
>>> Neither of those tools create code and install it in the VM. I don’t think 
>>> a fine grained SecurityManager policy makes sense for a VM compiler since 
>>> it could subvert security by compiling/installing malicious code. That is, 
>>> a VM compiler has to be a trusted component. Keep in mind that user code 
>>> cannot get to jdk.vm.compiler.
>> 
>> My question is not about granting fine-grained permissions vs 
>> AllPermissions.  I expect jdk.vm.compiler is used with jdk.aot which does 
>> not run with security manager.
>> 
>> If jdk.vm.compiler is run with VM as JIT and with security manager, the user 
>> can set -Djava.security.policy to a security policy configuring the 
>> permission for jdk.vm.compiler.
>> 
>> grant codeBase "jrt:/jdk.vm.compiler" {
>>  permission java.security.AllPermission;
>> };
>> 
>> If -XX:+BootstrapJVMCI -XX:+UseJVMCICompiler are supported, the other 
>> question I have is which loader jdk.vm.compiler should be defined?
> 
> I’m no expert on class loaders, but I would guess the same loader as 
> jdk.vm.ci.
> 
> -Doug

Re: JEP 158 support for JIT

[Christian Thalinger]

> On Jan 2, 2017, at 6:08 PM, Yasumasa Suenaga  wrote:
> 
> Thanks Vladimir,
> 
>> Definitely not in JDK 9. And I can't say when it could be done or done at 
>> all.
> 
> I hope this feature will be implemented ASAP.

You can always contribute an implementation.  Shouldn’t be too difficult.

> 
> 
> Yasumasa
> 
> 
> On 2017/01/03 12:00, Vladimir Kozlov wrote:
>> On 1/2/17 6:33 PM, Yasumasa Suenaga wrote:
>>> Hi all,
>>> 
>>> Java 9 has JEP 158: Unified JVM Logging.
>>> This JEP describes that existing 'tty->print...' logging should use unified 
>>> logging as output. However, C2 compiler does
>>> not seem to use it.
>>> 
>>> Do you have any plan to use JEP 158 in JIT codes?
>> 
>> Definitely not in JDK 9. And I can't say when it could be done or done at 
>> all.
>> 
>> Regards,
>> Vladimir
>> 
>>> 
>>> I uploaded Unified JVM logging viewer to GitHub [1].
>>> I want to draw chart(s) or list all JIT'ed methods on it if possible.
>>> (Especially I want to get log from PrintCompilation and PrintIntrinsics 
>>> through Unified JVM logging)
>>> 
>>> 
>>> Thanks,
>>> 
>>> Yasumasa
>>> 
>>> 
>>> [1] https://github.com/YaSuenag/ulviewer
>>> 

Re: [9] RFR(M) 8166416: [AOT] Integrate JDK build changes and launcher 'jaotc' for AOT compiler

[Christian Thalinger]

> On Oct 28, 2016, at 11:23 AM, Mandy Chung  wrote:
> 
>> 
>> On Oct 28, 2016, at 1:52 PM, Vladimir Kozlov  
>> wrote:
>> 
>> Thank you, Mandy
>> 
>> On 10/28/16 1:31 PM, Mandy Chung wrote:
>>> 
 On Oct 26, 2016, at 5:45 PM, Vladimir Kozlov  
 wrote:
 
 http://cr.openjdk.java.net/~kvn/aot/hs.make.webrev/
>>> 
>>> make/gensrc/Gensrc-jdk.vm.compiler.gmk
>>>  This generates module-info.java.extra at build time to augment 
>>> module-info.java with `uses` and `provides`. module-info.java.extra is 
>>> expected to be checked in the source.
>>> 
>>>  Do you expect the list of `uses` and `provides` are often changed?
>> 
>> Yes, Graal code changes frequently and  we need to run annotation processor 
>> to generate these dependencies for each jdk.vm.compiler (Graal) module build.
>> 
> 
> When the OptionDescriptors list is changed, we can run the tool (you can add 
> a make target if it helps) to generate the list of `uses` and `provides`.  
> Then update module-info.java in the source together with the changes.  That 
> would work too, right?

Why can’t we have it generated?  It’s much more convenient and less maintenance 
overhead.  There needs to be a good reason to give this up.

> 
>>> The alternative is to declare `uses` and `provides` in module-info.java in 
>>> the source repo so that a reader can see the module descriptor content 
>>> without needing to do a build.  A test to detect if the module-info.java is 
>>> out-of-sync with the annotated options.  In addition a make target to 
>>> generate the list of `use` and `provides` can be used to generate 
>>> module-info.java to be included in any change in the annotated options list.
>> 
>> What is "a reader”?
> 
> A person reading the code.
> 
>> And how to check "out-of-sync" without running 'processor’?
> 
> I meant a regression test that will run the annotation processor and compare 
> with module-info.class in the image (using java.lang.module.ModuleDescriptor 
> API)
> 
> Mandy
> 
>> Sorry, I am not familiar with all this modules build process.
>> 
>> Thanks,
>> Vladimir
>> 
>>> 
>>> Mandy

Re: [9] RFR(M) 8166416: [AOT] Integrate JDK build changes and launcher 'jaotc' for AOT compiler

[Christian Thalinger]

> On Oct 27, 2016, at 8:12 AM, Vladimir Kozlov <vladimir.koz...@oracle.com> 
> wrote:
> 
> On 10/27/16 10:55 AM, Christian Thalinger wrote:
>> 
>>> On Oct 27, 2016, at 2:40 AM, Erik Joelsson <erik.joels...@oracle.com> wrote:
>>> 
>>> 
>>> 
>>> On 2016-10-27 02:45, Vladimir Kozlov wrote:
>>>> AOT JEP:
>>>> https://bugs.openjdk.java.net/browse/JDK-8166089
>>>> Subtask:
>>>> https://bugs.openjdk.java.net/browse/JDK-8166416
>>>> Webrev:
>>>> http://cr.openjdk.java.net/~kvn/aot/top.webrev/
>>> hotspot.m4: 296: Comment is misleading. Should just be removed.
>>> 
>>> CompileJavaModules.gmk: Use of -g flag for java compilation is controlled 
>>> globally. Please remove.
>> 
>> There is a reason for that.  Some debugging related Graal code makes use of 
>> classfile information to provide better information.  Since this is Java and 
>> not C++ it *is* possible to have pleasant debugging experience even in 
>> product builds.  I want this to be there.
> 
> Chris, do we need -g for JVMCI module too for that?

Yes.

> 
> And do we need -Xlint:-exports for jdk.vm.compiler (Graal)? FOR JVMCI it was 
> added by:
> 
> http://hg.openjdk.java.net/jdk9/hs/rev/81435a812f59 
> <http://hg.openjdk.java.net/jdk9/hs/rev/81435a812f59>

I can’t remember what the reason for -Xlint was but… wait.  That was added 3 
weeks ago.  No clue then.

> 
> Thanks,
> Vladimir
> 
>> 
>>> 
>>> Main.gmk: buildtools-hotspot should be declared inside the 
>>> CREATING_BUILDJDK conditional like all other buildtools targets.
>>> 
>>>> http://cr.openjdk.java.net/~kvn/aot/jdk.webrev/
>>> The extra exports from java.base needs to go in a new 
>>> jdk/src/java.base/share/classes/module-info.java.extra since the module 
>>> jdk.vm.compiler is optional.
>>>> http://cr.openjdk.java.net/~kvn/aot/hs.make.webrev/
>>> Lib-jdk.aot.gmk: Please inline LDFLAGS and LIBS and add $(LIBS_JDKLIB) to 
>>> LIBS since that will provide -lc on Solaris automatically. No need to set 
>>> DEBUG_SYMBOLS or STRIP_SYMBOLS as the defaults should be correct and 
>>> controlled globally.
>>> 
>>> /Erik
>>>> 
>>>> Please, review build changes for AOT.  Only Linux/x64 platform is 
>>>> supported. 'jaotc' and AOT part of Hotspot will be build only on Linux/x64.
>>>> 
>>>> Changes include new 'jaotc' launcher, makefile changes to build 
>>>> jdk.vm.compiler (Graal) and jdk.aot modules used by 'jaotc'.
>>>> Both modules sources are located in Hotspot: hotspot/src/jdk.aot and 
>>>> hotspot/src/jdk.vm.compiler.
>>>> 'jaotc' requires installed libelf package on a system to build native part 
>>>> of 'jaotc'. It is used to generated AOT shared libraries (.so) as result 
>>>> of AOT compilation.
>>>> 
>>>> Hotspot makefile changes will be pushed together with Hotspot AOT changes.
>>>> 
>>>> Thanks,
>>>> Vladimir

Re: [9] RFR(M) 8166416: [AOT] Integrate JDK build changes and launcher 'jaotc' for AOT compiler

[Christian Thalinger]

> On Oct 27, 2016, at 2:40 AM, Erik Joelsson  wrote:
> 
> 
> 
> On 2016-10-27 02:45, Vladimir Kozlov wrote:
>> AOT JEP:
>> https://bugs.openjdk.java.net/browse/JDK-8166089
>> Subtask:
>> https://bugs.openjdk.java.net/browse/JDK-8166416
>> Webrev:
>> http://cr.openjdk.java.net/~kvn/aot/top.webrev/
> hotspot.m4: 296: Comment is misleading. Should just be removed.
> 
> CompileJavaModules.gmk: Use of -g flag for java compilation is controlled 
> globally. Please remove.

There is a reason for that.  Some debugging related Graal code makes use of 
classfile information to provide better information.  Since this is Java and 
not C++ it *is* possible to have pleasant debugging experience even in product 
builds.  I want this to be there.

> 
> Main.gmk: buildtools-hotspot should be declared inside the CREATING_BUILDJDK 
> conditional like all other buildtools targets.
> 
>> http://cr.openjdk.java.net/~kvn/aot/jdk.webrev/
> The extra exports from java.base needs to go in a new 
> jdk/src/java.base/share/classes/module-info.java.extra since the module 
> jdk.vm.compiler is optional.
>> http://cr.openjdk.java.net/~kvn/aot/hs.make.webrev/
> Lib-jdk.aot.gmk: Please inline LDFLAGS and LIBS and add $(LIBS_JDKLIB) to 
> LIBS since that will provide -lc on Solaris automatically. No need to set 
> DEBUG_SYMBOLS or STRIP_SYMBOLS as the defaults should be correct and 
> controlled globally.
> 
> /Erik
>> 
>> Please, review build changes for AOT.  Only Linux/x64 platform is supported. 
>> 'jaotc' and AOT part of Hotspot will be build only on Linux/x64.
>> 
>> Changes include new 'jaotc' launcher, makefile changes to build 
>> jdk.vm.compiler (Graal) and jdk.aot modules used by 'jaotc'.
>> Both modules sources are located in Hotspot: hotspot/src/jdk.aot and 
>> hotspot/src/jdk.vm.compiler.
>> 'jaotc' requires installed libelf package on a system to build native part 
>> of 'jaotc'. It is used to generated AOT shared libraries (.so) as result of 
>> AOT compilation.
>> 
>> Hotspot makefile changes will be pushed together with Hotspot AOT changes.
>> 
>> Thanks,
>> Vladimir
> 

Re: [9] RFR (xs) 8168317: [JVMCI] use reflection instead of jdk 9 Module API in Services.java

[Christian Thalinger]

> On Oct 24, 2016, at 9:01 PM, Doug Simon <doug.si...@oracle.com> wrote:
> 
>> 
>> On 24 Oct 2016, at 23:24, Christian Thalinger <cthalin...@twitter.com> wrote:
>> 
>> 
>>> On Oct 24, 2016, at 3:48 AM, Erik Joelsson <erik.joels...@oracle.com> wrote:
>>> 
>>> Adding build-dev, which should be included when discussing build issues. 
>>> For any new readers, please see [1] for the full discussion.
>>> In theory it is possible to compile against and run on the exploded image 
>>> during the build, but I do not recommend it. Igor is correct in the build 
>>> team being against that design. The rationale is that it adds a lot of 
>>> complexity to the build. The exploded image cannot be safely run until all 
>>> java modules have been compiled as it introduces races. Maintaining the 
>>> build with such a construct will be very brittle when other changes are 
>>> made. We did allow the gensrc for jdk.vm.ci to run in this way for a short 
>>> time, since it was only supported on Linux x64, where these races are 
>>> rarer, but if this would ever need to be built on Windows, we would be in 
>>> trouble quickly. Luckily, jdk.vm.ci was able to refactor away from needing 
>>> this annotation processing for that module. 
>>> I certainly prefer the reflection solution proposed here, but find it sad 
>>> that it's needed. 
>>> 
>> 
>> The problem I have with this solution is that jdk.vm.ci code is now 
>> restricted to JDK N-1 code.  This might be ok right now because we are able 
>> to work around that issue with reflection but when important features like 
>> Valhalla come around this is a problem.
>> 
>> Value types will be hugely important for JVMCI and its compilers and we 
>> simply cannot just skip one JDK release just because the build system can’t 
>> handle it.
> 
> I agree. However, I suspect large parts of the JDK will also want to leverage 
> Valhalla so I’m guessing the build problem will have to be solved anyway at 
> some point.

Not necessarily.  Java modules can and are using Java features of JDK N because 
they are compiled with a “new javac” that gets compiled before any Java modules 
are compiled:

# The generate old bytecode javac setup uses the new compiler to compile for the
# boot jdk to generate tools that need to be run with the boot jdk.
# Thus we force the target bytecode to the previous JDK version.
# Add -Xlint:-options to avoid the warning about not setting -bootclasspath. 
Since
# it's running on the boot jdk, the default bootclasspath is correct.
$(eval $(call SetupJavaCompiler,GENERATE_OLDBYTECODE, \

# The generate new bytecode javac setup uses the new compiler to compile for the
# new jdk. This new bytecode might only be possible to run using the new jvm.
$(eval $(call SetupJavaCompiler,GENERATE_JDKBYTECODE, \

So this will just work as it does already today.  Annotation processors need to 
be handled differently because their code is actually executed.

> Maybe it worth filing a tracking bug to revert these changes at that time.
> 
> -Doug
> 
>> 
>>> 
>>> /Erik
>>> 
>>> [1] 
>>> http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/2016-October/024743.html
>>> 
>>> On 2016-10-19 21:54, Igor Veresov wrote:
>>>> 
>>>>> On Oct 19, 2016, at 12:47 PM, Christian Thalinger 
>>>>> <cthalin...@twitter.com> wrote:
>>>>> 
>>>>> 
>>>>>> On Oct 19, 2016, at 9:40 AM, Vladimir Kozlov 
>>>>>> <vladimir.koz...@oracle.com> wrote:
>>>>>> 
>>>>>> https://bugs.openjdk.java.net/browse/JDK-8168317
>>>>>> 
>>>>>> webrev:
>>>>>> http://cr.openjdk.java.net/~kvn/8168317/webrev/
>>>>>> 
>>>>>> When Graal is built as part of JDK it requires first to build an 
>>>>>> annotation processor using boot jdk 8.
>>>>>> After JDK-8167180 changes Services class is referenced by annotation 
>>>>>> processor but the code is using jdk 9 Module API and it can't be used 
>>>>>> with jdk 8.
>>>>> 
>>>>> I left a comment in the bug: Permalink
>>>>> 
>>>>> Basically, it should be possible to use the newly built javac to compile 
>>>>> the annotation processors.  Erik?
>>>> 
>>>> It’s not only about compilation it’s about running it on the bootstrap 
>>>> JDK, which in currently 8.
>>>> 
>>>> igor
>>>> 
>>>>> 
>>>>> Can you paste or upload the .gmk file?
>>>>> 
>>>>>> 
>>>>>> Use reflection instead of Module API and use code only for running with 
>>>>>> jdk 9.
>>>>>> 
>>>>>> Testing with JPRT and JDK build of Graal.
>>>>>> 
>>>>>> Thanks,
>>>>>> Vladimir

Re: [9] RFR (xs) 8168317: [JVMCI] use reflection instead of jdk 9 Module API in Services.java

[Christian Thalinger]

> On Oct 24, 2016, at 3:48 AM, Erik Joelsson <erik.joels...@oracle.com> wrote:
> 
> Adding build-dev, which should be included when discussing build issues. For 
> any new readers, please see [1] for the full discussion.
> In theory it is possible to compile against and run on the exploded image 
> during the build, but I do not recommend it. Igor is correct in the build 
> team being against that design. The rationale is that it adds a lot of 
> complexity to the build. The exploded image cannot be safely run until all 
> java modules have been compiled as it introduces races. Maintaining the build 
> with such a construct will be very brittle when other changes are made. We 
> did allow the gensrc for jdk.vm.ci to run in this way for a short time, since 
> it was only supported on Linux x64, where these races are rarer, but if this 
> would ever need to be built on Windows, we would be in trouble quickly. 
> Luckily, jdk.vm.ci was able to refactor away from needing this annotation 
> processing for that module. 
> I certainly prefer the reflection solution proposed here, but find it sad 
> that it's needed. 
> 

The problem I have with this solution is that jdk.vm.ci code is now restricted 
to JDK N-1 code.  This might be ok right now because we are able to work around 
that issue with reflection but when important features like Valhalla come 
around this is a problem.

Value types will be hugely important for JVMCI and its compilers and we simply 
cannot just skip one JDK release just because the build system can’t handle it.

> /Erik
> 
> [1] 
> http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/2016-October/024743.html
>  
> <http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/2016-October/024743.html>
> 
> On 2016-10-19 21:54, Igor Veresov wrote:
>> 
>>> On Oct 19, 2016, at 12:47 PM, Christian Thalinger <cthalin...@twitter.com 
>>> <mailto:cthalin...@twitter.com>> wrote:
>>> 
>>> 
>>>> On Oct 19, 2016, at 9:40 AM, Vladimir Kozlov <vladimir.koz...@oracle.com 
>>>> <mailto:vladimir.koz...@oracle.com>> wrote:
>>>> 
>>>> https://bugs.openjdk.java.net/browse/JDK-8168317 
>>>> <https://bugs.openjdk.java.net/browse/JDK-8168317>
>>>> 
>>>> webrev:
>>>> http://cr.openjdk.java.net/~kvn/8168317/webrev/ 
>>>> <http://cr.openjdk.java.net/%7Ekvn/8168317/webrev/>
>>>> 
>>>> When Graal is built as part of JDK it requires first to build an 
>>>> annotation processor using boot jdk 8.
>>>> After JDK-8167180 changes Services class is referenced by annotation 
>>>> processor but the code is using jdk 9 Module API and it can't be used with 
>>>> jdk 8.
>>> 
>>> I left a comment in the bug: Permalink 
>>> <https://bugs.openjdk.java.net/browse/JDK-8168317?focusedCommentId=14013733=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14013733>
>>> 
>>> Basically, it should be possible to use the newly built javac to compile 
>>> the annotation processors.  Erik?
>> 
>> It’s not only about compilation it’s about running it on the bootstrap JDK, 
>> which in currently 8.
>> 
>> igor
>> 
>>> 
>>> Can you paste or upload the .gmk file?
>>> 
>>>> 
>>>> Use reflection instead of Module API and use code only for running with 
>>>> jdk 9.
>>>> 
>>>> Testing with JPRT and JDK build of Graal.
>>>> 
>>>> Thanks,
>>>> Vladimir
>>> 
>> 
> 

Re: RFR[9u-dev]: 8150900: Implement diagnostic_pd

[Christian Thalinger]

src/share/vm/runtime/globals.hpp

-  develop_pd(bool, ImplicitNullChecks,  \
+  diagnostic_pd(bool, ImplicitNullChecks,  
\
   "Generate code for implicit null checks") \
Align the \

> On May 10, 2016, at 1:47 AM, Cheleswer Sahu  wrote:
> 
> Hi, 
> I need one reviewer (R) to review these changes before pushing in JDK9.  Can 
> somebody please review the changes.
> 
> Regards,
> Cheleswer
> 
>> -Original Message-
>> From: Kevin Walls
>> Sent: Friday, May 06, 2016 3:53 PM
>> To: Cheleswer Sahu; Gerard Ziemski
>> Cc: serviceability-dev@openjdk.java.net; hotspot-runtime-
>> d...@openjdk.java.net
>> Subject: Re: RFR[9u-dev]: 8150900: Implement diagnostic_pd
>> 
>> 
>> Thanks Cheleswer, looks good to me too, have been over the macros as
>> much as I can!
>> 
>> Thanks
>> Kevin
>> 
>> 
>> 
>> On 03/05/2016 07:34, Cheleswer Sahu wrote:
>>> Hi Gerard,
>>> 
>>> 
 -Original Message-
 From: Gerard Ziemski
 Sent: Monday, May 02, 2016 9:07 PM
 To: Cheleswer Sahu
 Cc: hotspot-runtime-...@openjdk.java.net; serviceability-
 d...@openjdk.java.net
 Subject: Re: RFR[9u-dev]: 8150900: Implement diagnostic_pd
 
 hi Cheleswer,
 
 
 #1 Shouldn’t the following files be modified as well? :
 
 open:
 
 src/cpu/sparc/vm/globals_sparc.hpp
 src/cpu/x86/vm/globals_x86.hpp
 src/cpu/zero/vm/globals_zero.hpp
 
 closed:
 
 cpu/arm/vm/globals_arm.hpp
>>> I have implemented  "diagnostic_pd" using "product_pd" as a reference
>> implementation. "product_pd" is not implemented for " ARCH_FLAGS ",
>> therefore I have also not implemented "diagnostic_pd" for "ARCH_FLAGS"
>> type.
>>> 
 share/vm/runtime/globals_ext.hpp
 share/vm/runtime/os_ext.hpp
>>> These 2 files are under closed repository, so I have initiated a separate
>> internal review request for those changes.
>>> 
 
 #2 Bunch of header files need to be updated with 2016 for Copyright:
 
 /*
 - * Copyright (c) 2011, 2015, Oracle and/or its affiliates. All rights 
 reserved.
 + * Copyright (c) 2011, 2016, Oracle and/or its affiliates. All rights 
 reserved.
  * ORACLE PROPRIETARY/CONFIDENTIAL.  Use is subject to license terms.
  */
 
 
>>> I agree, I will update the copyright headers.
>>> 
 #3 What tests have you run? Did you do:
 
 a) JPRT hotspot
 b) RBT hs-nightly-runtime
 
>>> I have run JPRT hostspot tests for this. It shows no error.
>>> 
 Please email me if you need help with those.
 
 
 #4 Just heads up that I will be shortly asking for review of my fix
 (https://bugs.openjdk.java.net/browse/JDK-8073500), which touches
 many of the same file, so one of us will have a tricky merge
 
>>> Thanks for informing about this.
>>> 
>>> 
>>> Regards,
>>> Cheleswer
>>> 
 cheers
 
> On May 2, 2016, at 4:51 AM, Cheleswer Sahu
  wrote:
> Hi,
> 
> 
> 
> Please review the code changes for
 https://bugs.openjdk.java.net/browse/JDK-8150900.
> 
> 
> Webrev Link: http://cr.openjdk.java.net/~csahu/8150900/webrev.00/
> 
> 
> 
> Enhancement Brief:  A new variant of flag "diagnostic_pd" is
>> implemented.
 All flags which are diagnostic in nature and platform dependent can
 be placed
> under this variant. These flags can be enable using  "-
 XX:+UnlockDiagnosticVMOptions".
> At present I have placed 4 flags under "diagnostic_pd"
> 
> 1.1. InitArrayShortSize
> 
> 2.2. ImplicitNullChecks
> 
> 3.3. InlineFrequencyCount
> 
> 4.4. PostLoopMultiversioning
> 
> 
> 
> 
> 
> Regards,
> 
> Cheleswer
>> 

Re: [9] RFR: 8152207: Perform array bound checks while getting a length of bytecode instructions

[Christian Thalinger]

> On May 4, 2016, at 1:48 PM, Artem Smotrakov  
> wrote:
> 
> Hello,
> 
> Please review two small patches for jdk and hotspot repos which add array 
> bound checks to functions which return a length of bytecode instruction.
> 
> Please see details in https://bugs.openjdk.java.net/browse/JDK-8152207
> 
> http://cr.openjdk.java.net/~asmotrak/8152207/jdk/webrev.00/
> http://cr.openjdk.java.net/~asmotrak/8152207/hotspot/webrev.00/

  static boolis_defined (int  code){ return 0 <= code && code < 
number_of_codes && flags(code, false) != 0; }
+  static int length_for (Code code){ return 0 <= code && code 
< number_of_codes ? _lengths[code] & 0xF : -1; }
+  static int wide_length_for(Code code){ return 0 <= code && code 
< number_of_codes ? _lengths[code]  >> 4 : -1; }
You should factor the bound check into a separate method.

> 
> Artem

Re: ClassValue perf?

[Christian Thalinger]

> On May 6, 2016, at 4:48 AM, Michael Haupt  wrote:
> 
> Hi Peter,
> 
> thank you. I've run the full benchmark in my setup and uploaded the updated 
> cumulative results to http://cr.openjdk.java.net/~mhaupt/8031043/ 
> .
> 
> The benchmark indeed shows that this latest addition to the group slows down 
> random and sequential access, especially for small numbers of values and 
> classes. The OpenJDK tests are fine; I'm running a batch of internal tests as 
> well.
> 
> Given that one concern with this issue, next to reducing footprint, was to 
> optimise for the single-value case, I'm still a bit hesitant even though the 
> sheer amount of code reduction is impressive. I'll evaluate further.

The main motivation to optimize for the single-value use case is Graal but it’s 
not super important.  Graal solved this issue in a different way and it’s 
questionable Graal would go back using ClassValue so don’t worry too much about 
it.

> 
> Best,
> 
> Michael
> 
>> Am 05.05.2016 um 17:21 schrieb Peter Levart > >:
>> 
>> Hi Michael,
>> 
>> 
>> On 05/04/2016 06:02 PM, Michael Haupt wrote:
>>> Hi Peter,
>>> 
>>> thank you for chiming in again! :-) I'll look at this in depth on Friday.
>> 
>> Good. Because I found bugs in expunging logic and a discrepancy of behavior 
>> when a value is installed concurrently by some other thread and then later 
>> removed while the 1st thread is still calculating the value. Current 
>> ClassValue re-tries the computation until it can make sure there were no 
>> concurrent changes to the entry during its computation. I fixed both things 
>> and verified that the behavior is now the same:
>> 
>> 
>> http://cr.openjdk.java.net/~plevart/misc/ClassValue.Alternative2/webrev.02/ 
>> 
>> 
>> Regards, Peter
> 
> 
> -- 
> 
>  
> Dr. Michael Haupt | Principal Member of Technical Staff
> Phone: +49 331 200 7277 | Fax: +49 331 200 7561
> Oracle Java Platform Group | LangTools Team | Nashorn
> Oracle Deutschland B.V. & Co. KG | Schiffbauergasse 14 | 14467 Potsdam, 
> Germany
> 
> ORACLE Deutschland B.V. & Co. KG | Hauptverwaltung: Riesstraße 25, D-80992 
> München
> Registergericht: Amtsgericht München, HRA 95603
> 
> Komplementärin: ORACLE Deutschland Verwaltung B.V. | Hertogswetering 163/167, 
> 3543 AS Utrecht, Niederlande
> Handelsregister der Handelskammer Midden-Nederland, Nr. 30143697
> Geschäftsführer: Alexander van der Ven, Jan Schultheiss, Val Maher
>     Oracle is committed to developing 
> practices and products that help protect the environment
> 
> ___
> mlvm-dev mailing list
> mlvm-dev@openjdk.java.net 
> http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev 
> 
___
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

Re: RFR(S): 8154209: Remove client VM from default JIB profile on windows-x86 and linux-x86

[Christian Thalinger]

> On Apr 14, 2016, at 6:41 AM, Mikael Vidstedt  
> wrote:
> 
> 
> Please review the following change which removes the "client" VM from the 
> default JIB build profile

Is there some public documentation about JIB?  A quick search only showed a few 
JBS bugs that mention JIB.

> on windows-x86 and linux-x86:
> 
> Bug: https://bugs.openjdk.java.net/browse/JDK-8154209
> Webrev (top): http://cr.openjdk.java.net/~mikael/webrevs/8154209/webrev.01/
> Webrev (hotspot): 
> http://cr.openjdk.java.net/~mikael/webrevs/8154209/webrev.01/hotspot/webrev/
> 
> 
> When not including the client VM, the build system automatically creates a 
> jvm.cfg which makes -client an alias for -server. At some point in the future 
> we may choose to output a warning and/or refuse to start up if -client is 
> specified, but at least for now silently falling back on the -server VM seems 
> appropriate.
> 
> The test/runtime/SharedArchiveFile/DefaultUseWithClient.java test assumes 
> that CDS is always compiled in and enabled in the -client VM on windows-x86. 
> Since -client will fall back on -server that is no longer true, so the test 
> needs to be updated. I added an @ignore and filed the following issue to 
> track fixing the test:
> 
> https://bugs.openjdk.java.net/browse/JDK-8154204
> 
> 
> Testing:
> 
> In addition to a standard JPRT push job, Christian Tornqvist helped me run 
> the runtime nightly tests and apart from the above mentioned test all tests 
> were successful.
> 
> Cheers,
> Mikael
> 

Re: RFR: JDK-8152666: The new Hotspot Build System

[Christian Thalinger]

> On Apr 5, 2016, at 11:14 PM, Erik Joelsson <erik.joels...@oracle.com> wrote:
> 
> Hello,
> 
> I assume the mx projects are for Java code or do they also generate projects 
> for native?

Also native.  Look at the screenshots I posted.  Particularly this one:

http://cr.openjdk.java.net/~twisti/8139921/Screen%20Shot%202015-11-10%20at%202.18.20%20PM.png
 
<http://cr.openjdk.java.net/~twisti/8139921/Screen%20Shot%202015-11-10%20at%202.18.20%20PM.png>

> The new top level target is only meant to replace the old Visual Studio 
> project generator, at least for now.
> 
> /Erik
> 
> On 2016-04-06 03:23, Christian Thalinger wrote:
>> 
>>> On Apr 5, 2016, at 8:10 AM, Daniel D. Daugherty 
>>> <daniel.daughe...@oracle.com <mailto:daniel.daughe...@oracle.com>> wrote:
>> 
>> …
>> 
>>> make/Main.gmk
>>>No comments other than the 'hotspot-ide-project' target
>>>looks interesting...
>> 
>> Btw. there is already support to generate IDE configurations today via mx:
>> 
>> https://wiki.openjdk.java.net/display/Graal/Instructions 
>> <https://wiki.openjdk.java.net/display/Graal/Instructions>
>> 
>> integrated with:
>> 
>> https://bugs.openjdk.java.net/browse/JDK-8139921 
>> <https://bugs.openjdk.java.net/browse/JDK-8139921>
>> 
>> One main advantage, as I pointed out in the review, is that it also includes 
>> generated files so there are no unresolved includes or methods anymore:
>> 
>> http://mail.openjdk.java.net/pipermail/hotspot-dev/2015-November/020626.html 
>> <http://mail.openjdk.java.net/pipermail/hotspot-dev/2015-November/020626.html>
>> 
>> I’m using this every day.
> 

Re: RFR: JDK-8152666: The new Hotspot Build System

[Christian Thalinger]

> On Apr 5, 2016, at 8:10 AM, Daniel D. Daugherty  
> wrote:

…

> make/Main.gmk
>No comments other than the 'hotspot-ide-project' target
>looks interesting...

Btw. there is already support to generate IDE configurations today via mx:

https://wiki.openjdk.java.net/display/Graal/Instructions

integrated with:

https://bugs.openjdk.java.net/browse/JDK-8139921

One main advantage, as I pointed out in the review, is that it also includes 
generated files so there are no unresolved includes or methods anymore:

http://mail.openjdk.java.net/pipermail/hotspot-dev/2015-November/020626.html

I’m using this every day.

Re: RFR: JDK-8151656: Minor tweaks to old Hotspot build to ease comparison with new

[Christian Thalinger]

> On Mar 15, 2016, at 10:51 PM, Erik Joelsson <erik.joels...@oracle.com> wrote:
> 
> The __FILE__ macro is built into the compiler and evaluates to the file name 
> of the current source file. It's intended use is for debug messages. In the 
> rest of the JDK we eliminated its use completely by providing a macro called 
> THIS_FILE that evaluates to the basename.ext of the source file being 
> compiled. In Hotspot it's still used and it's used in header files, which 
> makes the THIS_FILE solution invalid.
> 
> For most of our compilers, the format of the path is based on how the file 
> was presented on the command line. This matters to us since the old Hotspot 
> build uses relative paths to the generated source and header files and the 
> new build uses absolute paths for all source files. Also, the directory 
> structure in the output directory differ. This means that for certain object 
> files, the constant strings resulting from the use of __FILE__ differ, both 
> in contents and length. The difference in length causes alignment differences 
> in the objects and the resulting libraries.

This is actually a very good point.  Alignment differences can cause 
performance differences.  Sometimes hard to track down and almost impossible to 
reproduce.

> 
> When creating the new build, we are trying very hard to make sure we are 
> still building the same thing. One way to verify this is to compare various 
> aspects of the built binaries. When possible, this is far easier than running 
> a very large amount of tests and it gives us a large amount of confidence in 
> our changes. The alignment differences caused by different lengths of 
> constant strings severely limits the amount of clean comparisons we can do.
> 
> The particular fix here changes how we generate the files from the ADLC tool. 
> The tool already generates #line directives which helps the compiler figure 
> out which actual source file and line parts of the generated files came from. 
> Basically overriding what __FILE__ and __LINE__ will evaluate to. Some of 
> these lines need a bit of post processing, and for that, the build already 
> uses awk to rewrite them. The current awk script looks for lines like this, 
> which the ADLC tool prints when it doesn't know the correct source file:
> 
> #line 99
> 
> and rewrites it to
> 
> #line   file>
> 
> I changed this to instead rewrite to:
> 
> #line  

You removed the +1.  Why was it used in the first place?

> 
> I also added an initial #line first in each file.
> 
> #line 1 
> 
> With these changes, we can do very clean comparisons on Linux, Solaris and 
> Macosx. This helps the new hotspot build immensely, but will also be good in 
> the future as we have a very convenient way of verifying build changes that 
> shouldn't affect the built output.

Now this all makes sense.  Thanks.  The change looks good to me.

> 
> /Erik
> 
> On 2016-03-15 22:02, Christian Thalinger wrote:
>>> On Mar 15, 2016, at 12:06 AM, Erik Joelsson <erik.joels...@oracle.com> 
>>> wrote:
>>> 
>>> Any chance I could get a second review on this? Preferably from the hotspot 
>>> team.
>> It’s not quite obvious to me what the __FILE__ change is.  Maybe an example 
>> would help.
>> 
>>> /Erik
>>> 
>>> On 2016-03-11 18:16, Erik Joelsson wrote:
>>>> In preparation for the new Hotspot build, there are a few changes to the 
>>>> old build that needs to happen first. These changes should be harmless, 
>>>> but do impact the generated binaries some. These changes are:
>>>> 
>>>> * Standardizing sort order for objects on link command line on Windows to 
>>>> a locale independent order.
>>>> * Working around compare differences caused by the __FILE__ macro in 
>>>> certain generated files by overriding the value of __FILE__ in those files.
>>>> 
>>>> By making these changes first and separate from introducing the new build 
>>>> we reduce the potential impact of when we do introduce the new build.
>>>> 
>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8151656
>>>> Webrev: http://cr.openjdk.java.net/~erikj/8151656/index.html
>>>> 
>>>> /Erik
> 

Re: RFR: JDK-8151656: Minor tweaks to old Hotspot build to ease comparison with new

[Christian Thalinger]

> On Mar 15, 2016, at 12:06 AM, Erik Joelsson  wrote:
> 
> Any chance I could get a second review on this? Preferably from the hotspot 
> team.

It’s not quite obvious to me what the __FILE__ change is.  Maybe an example 
would help.

> 
> /Erik
> 
> On 2016-03-11 18:16, Erik Joelsson wrote:
>> In preparation for the new Hotspot build, there are a few changes to the old 
>> build that needs to happen first. These changes should be harmless, but do 
>> impact the generated binaries some. These changes are:
>> 
>> * Standardizing sort order for objects on link command line on Windows to a 
>> locale independent order.
>> * Working around compare differences caused by the __FILE__ macro in certain 
>> generated files by overriding the value of __FILE__ in those files.
>> 
>> By making these changes first and separate from introducing the new build we 
>> reduce the potential impact of when we do introduce the new build.
>> 
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8151656
>> Webrev: http://cr.openjdk.java.net/~erikj/8151656/index.html
>> 
>> /Erik
> 

Re: RFR (S) 8150180: String.value contents should be trusted

[Christian Thalinger]

> On Feb 19, 2016, at 1:22 PM, Aleksey Shipilev <aleksey.shipi...@oracle.com> 
> wrote:
> 
> On 02/20/2016 01:40 AM, Christian Thalinger wrote:
>>> On Feb 19, 2016, at 9:03 AM, John Rose <john.r.r...@oracle.com> wrote:
>>> On Feb 19, 2016, at 9:57 AM, Christian Thalinger 
>>> <christian.thalin...@oracle.com <mailto:christian.thalin...@oracle.com>> 
>>> wrote:
>>>> Why don’t you change the values to:
>>>> 
>>>>   static final byte LATIN1 = 1;
>>>>   static final byte UTF16  = 2;
> 
> We've been over this during Compact Strings development. The idea that
> John has below is related to our actual insights leading to 0/1 instead
> of 1/2. The best thing you can do with 1/2 is, apparently:
> 
>  int length() {
> return value.length >> (coder - 1);
>  }
> 
>  char charAt(int idx) {
> return getCharAt(idx * coder);// variant 1
> return getCharAt(idx << (coder - 1)); // variant 2
>  }
> 
> ...and you are better off not doing excess "-1" or
> non-strength-reducible multiplications in a very hot paths in String.
> 
> Anyhow, that ship had sailed, and any change in coder definitions would
> require to respin an awful lot of Compact String testing, and probably
> revalidating lots of premises in the code. This is good as a thought
> experiment, but not practical at this point in JDK 9 development.
> 
> 
>> But if coder is stable for both values the compiler can constant fold the 
>> if-statement for the shift value:
>> 
>>  int len = val.length >> (coder == LATIN1 ? 0 : 1);
>> 
>> That should produce the same code and we would avoid:
>> 
>> 143  * Constant-folding this field is handled internally in VM.
> 
> The constant-folding story is not the only story you should care about.
> 
> For most Strings, we do not know either String.value or String.coder
> statically. This particular constant-folding bit affects String
> concatenation with constants, but the solution to it cannot possibly
> regress an overwhelming case of non-constant Strings. Changing the coder
> designations *would* affect non-constant Strings.
> 
> I would guess that the comment on "coder" field throws a reader into
> thinking that @Stable is the answer to constant folding woes. But VM
> already trusts final fields in String (e.g. String.value.arraylength is
> folded, see JDK-8149813) -- as the part of TrustStaticNonFinalFields
> machinery, which hopefully some day would get exposed to other fields.
> Frozen arrays would hit the final (pun intended) nail into String.value
> folding. @Stable hack is doing that today; but that's a hack, for a very
> performance-sensitive corner in JDK.
> 
> Hopefully the rewritten comments spell it better:

That comment is better.

>  http://cr.openjdk.java.net/~shade/8150180/webrev.jdk.02/
>  http://cr.openjdk.java.net/~shade/8150180/webrev.hs.02/
> 
> Cheers,
> -Aleksey
> 
> 

Re: RFR (S) 8150180: String.value contents should be trusted

[Christian Thalinger]

> On Feb 19, 2016, at 9:03 AM, John Rose <john.r.r...@oracle.com> wrote:
> 
> On Feb 19, 2016, at 9:57 AM, Christian Thalinger 
> <christian.thalin...@oracle.com <mailto:christian.thalin...@oracle.com>> 
> wrote:
>> 
>> Why don’t you change the values to:
>> 
>>static final byte LATIN1 = 1;
>>static final byte UTF16  = 2;
> 
> 
> Not sure what you are asking, but here's my take on why 0,1 is the (slight) 
> winner.
> The values 0,1 are the log2 of the array element sizes 1,2, so they can be 
> used with shift instructions.
> With 1,2 they would have to be used with multiply instructions, or else 
> tweaked back to shift inputs.
> Most loops will speculate on the scale factor, but those loops which must 
> work with a non-constant scale will be slightly cleaner with 0,1.

I see what you are saying:

  int len = val.length >> coder;  // assume LATIN1=0/UTF16=1;

But if coder is stable for both values the compiler can constant fold the 
if-statement for the shift value:

  int len = val.length >> (coder == LATIN1 ? 0 : 1);

That should produce the same code and we would avoid:

143  * Constant-folding this field is handled internally in VM.

Re: RFR (S) 8150180: String.value contents should be trusted

[Christian Thalinger]

> On Feb 19, 2016, at 4:05 AM, Peter Levart  wrote:
> 
> Hi,
> 
> Just a stupid question.
> 
> Your comment in String:
> 
> 140  * @implNote Note this field is not {@link Stable}, because we want
> 141  * LATIN1 (0) coder to fold too. {@link Stable} would not allow that,
> 142  * as it reserves the default value as "not initialized" value.
> 143  * Constant-folding this field is handled internally in VM.
> 144  */
> 145 private final byte coder;

Why don’t you change the values to:

static final byte LATIN1 = 1;
static final byte UTF16  = 2;

?

> 
> 
> Couldn't @Stable final instance fields constant-fold the default value too in 
> general? I mean can't it be assumed that the final field has been set before 
> JIT kicks in?
> 
> Regards, Peter
> 
> On 02/19/2016 12:55 PM, Aleksey Shipilev wrote:
>> Hi,
>> 
>> Please review a simple performance improvement in Strings:
>>   https://bugs.openjdk.java.net/browse/JDK-8150180
>> 
>> In short, we want VM to trust constant String contents, so that
>> "Foo".charAt(0) is folded. As far as current Hotspot goes, this is only
>> achievable with @Stable -- we need to trust the array contents:
>>   http://cr.openjdk.java.net/~shade/8150180/webrev.jdk.01/
>> 
>> This, however, steps into the compiler bug caused by StringUTF16.getChar
>> intrinsic producing a mismatched load, that is then folded incorrectly.
>> So we instead rely on Java level folding in cases like these:
>>   http://cr.openjdk.java.net/~shade/8150180/webrev.hs.01/
>> 
>> ...and it works:
>>   http://cr.openjdk.java.net/~shade/8150180/notes.txt
>> 
>> While VM change looks like a workaround, Vladimir I. and me concluded
>> that @Stable folding code should just reject folding mismatched loads to
>> begin with. So, getChar intrinsic change is the actual fix. Vladimir I.
>> ought to add more assertions in @Stable folding code separately:
>>   https://bugs.openjdk.java.net/browse/JDK-8150186
>> 
>> Since this issue might trigger compiler bugs, I would like to push
>> through hs-comp to pass compiler nightlies.
>> 
>> Cheers,
>> -Aleksey
>> 
> 

Re: RFR [9] 8141615: Add new public methods to sun.reflect.ConstantPool

[Christian Thalinger]

Looks good.  Thanks.

> On Dec 16, 2015, at 1:13 AM, Konstantin Shefov <konstantin.she...@oracle.com> 
> wrote:
> 
> Christian
> 
> I have fixed the enum so it uses "ENUMENTRY(int)" format now and does linear 
> search.
> http://cr.openjdk.java.net/~kshefov/8141615/jdk/webrev.04/
> 
> -Konstantin
> 
> On 12/15/2015 08:36 PM, Christian Thalinger wrote:
>> 
>>> On Dec 14, 2015, at 11:11 PM, Konstantin Shefov 
>>> <konstantin.she...@oracle.com <mailto:konstantin.she...@oracle.com>> wrote:
>>> 
>>> Hi Christian
>>> 
>>> Thanks for reviewing, I have changed indents as you asked:
>>> 
>>> http://cr.openjdk.java.net/~kshefov/8141615/jdk/webrev.03 
>>> <http://cr.openjdk.java.net/%7Ekshefov/8141615/jdk/webrev.03>
>> 
>> Thanks.  I’m still not comfortable with the enum.  It would be great if we 
>> could get the values from the VM like in JVMCI:
>> 
>> http://hg.openjdk.java.net/jdk9/hs-comp/hotspot/file/c036c7f17e09/src/jdk.vm.ci/share/classes/jdk.vm.ci.hotspot/src/jdk/vm/ci/hotspot/HotSpotConstantPool.java#l101
>> 
>> but that would be overkill here.  But I would like to see the enum entries 
>> take the integer values as arguments, like here:
>> 
>> http://hg.openjdk.java.net/jdk9/hs-comp/hotspot/file/c036c7f17e09/src/jdk.vm.ci/share/classes/jdk.vm.ci.sparc/src/jdk/vm/ci/sparc/SPARCKind.java#l27
>> 
>> and either do a simple linear search to find the entry or build up a table 
>> like the HotSpotConstantPool example above.
>> 
>>> 
>>> -Konstantin
>>> 
>>> On 12/15/2015 06:23 AM, Christian Thalinger wrote:
>>>> 
>>>>> On Dec 11, 2015, at 1:54 AM, Konstantin Shefov 
>>>>> <konstantin.she...@oracle.com <mailto:konstantin.she...@oracle.com>> 
>>>>> wrote:
>>>>> 
>>>>> Hello
>>>>> 
>>>>> Please review the new version on the patch.
>>>>> 
>>>>> New webrev:
>>>>> Webrev hotspot: 
>>>>> http://cr.openjdk.java.net/~kshefov/8141615/hotspot/webrev.02 
>>>>> <http://cr.openjdk.java.net/%7Ekshefov/8141615/hotspot/webrev.02>
>>>>> Webrev jdk: http://cr.openjdk.java.net/~kshefov/8141615/jdk/webrev.02 
>>>>> <http://cr.openjdk.java.net/%7Ekshefov/8141615/jdk/webrev.02>
>>>> 
>>>> These newlines look ridiculous especially when it’s not even needed:
>>>> 
>>>> +  // Returns a class reference index for a method or a field.
>>>> +  public int  getClassRefIndexAt
>>>> + (int index) { return 
>>>> getClassRefIndexAt0 (constantPoolOop, index); }
>>>> 
>>>> Either fix the indent or just add them like regular methods should look 
>>>> like.
>>>> 
>>>>> 
>>>>> What has been changed:
>>>>> 1. Added tests for the new added methods.
>>>>> 2. Removed CP tag codes 100 - 105 from being passed to java and left only 
>>>>> codes from the open JVM spec 
>>>>> (https://docs.oracle.com/javase/specs/jvms/se8/html/jvms-4.html#jvms-4.4-140).
>>>>> 
>>>>> Thanks
>>>>> -Konstantin
>>>>> 
>>>>> On 11/27/2015 07:48 PM, Konstantin Shefov wrote:
>>>>>> Coleen,
>>>>>> Thanks for review
>>>>>> 
>>>>>> On 11/24/2015 07:33 PM, Coleen Phillimore wrote:
>>>>>>> 
>>>>>>> I have a couple preliminary comments.  First, there are no tests added 
>>>>>>> with all this new functionality.  Tests should be added with the 
>>>>>>> functionality changeset, not promised afterward.
>>>>>> I will add tests.
>>>>>>> Secondly, I do not like that JDK code knows the implementation details 
>>>>>>> of the JVM's constant pool tags.  These should be only for internal use.
>>>>>> The package "sun.reflect" is for internal use only, so it shouldn’t be a 
>>>>>> problem.
>>>>>>> My third comment is that I haven't looked carefully at this constant 
>>>>>>> pool implementation but it seems very unsafe if the class is redefined 
>>>>>>> and relies on an implementation detail in the JVM that can change.   I 
>>>>>>> will have more comments once I look more at the jvmti specification.
>>>>>&

Re: RFR [9] 8141615: Add new public methods to sun.reflect.ConstantPool

[Christian Thalinger]

> On Dec 14, 2015, at 11:11 PM, Konstantin Shefov 
> <konstantin.she...@oracle.com> wrote:
> 
> Hi Christian
> 
> Thanks for reviewing, I have changed indents as you asked:
> 
> http://cr.openjdk.java.net/~kshefov/8141615/jdk/webrev.03 
> <http://cr.openjdk.java.net/~kshefov/8141615/jdk/webrev.03>

Thanks.  I’m still not comfortable with the enum.  It would be great if we 
could get the values from the VM like in JVMCI:

http://hg.openjdk.java.net/jdk9/hs-comp/hotspot/file/c036c7f17e09/src/jdk.vm.ci/share/classes/jdk.vm.ci.hotspot/src/jdk/vm/ci/hotspot/HotSpotConstantPool.java#l101
 
<http://hg.openjdk.java.net/jdk9/hs-comp/hotspot/file/c036c7f17e09/src/jdk.vm.ci/share/classes/jdk.vm.ci.hotspot/src/jdk/vm/ci/hotspot/HotSpotConstantPool.java#l101>

but that would be overkill here.  But I would like to see the enum entries take 
the integer values as arguments, like here:

http://hg.openjdk.java.net/jdk9/hs-comp/hotspot/file/c036c7f17e09/src/jdk.vm.ci/share/classes/jdk.vm.ci.sparc/src/jdk/vm/ci/sparc/SPARCKind.java#l27
 
<http://hg.openjdk.java.net/jdk9/hs-comp/hotspot/file/c036c7f17e09/src/jdk.vm.ci/share/classes/jdk.vm.ci.sparc/src/jdk/vm/ci/sparc/SPARCKind.java#l27>

and either do a simple linear search to find the entry or build up a table like 
the HotSpotConstantPool example above.

> 
> -Konstantin
> 
> On 12/15/2015 06:23 AM, Christian Thalinger wrote:
>> 
>>> On Dec 11, 2015, at 1:54 AM, Konstantin Shefov 
>>> <konstantin.she...@oracle.com <mailto:konstantin.she...@oracle.com>> wrote:
>>> 
>>> Hello
>>> 
>>> Please review the new version on the patch.
>>> 
>>> New webrev:
>>> Webrev hotspot: 
>>> http://cr.openjdk.java.net/~kshefov/8141615/hotspot/webrev.02 
>>> <http://cr.openjdk.java.net/%7Ekshefov/8141615/hotspot/webrev.02>
>>> Webrev jdk: http://cr.openjdk.java.net/~kshefov/8141615/jdk/webrev.02 
>>> <http://cr.openjdk.java.net/%7Ekshefov/8141615/jdk/webrev.02>
>> 
>> These newlines look ridiculous especially when it’s not even needed:
>> 
>> +  // Returns a class reference index for a method or a field.
>> +  public int  getClassRefIndexAt
>> + (int index) { return 
>> getClassRefIndexAt0 (constantPoolOop, index); }
>> 
>> Either fix the indent or just add them like regular methods should look like.
>> 
>>> 
>>> What has been changed:
>>> 1. Added tests for the new added methods.
>>> 2. Removed CP tag codes 100 - 105 from being passed to java and left only 
>>> codes from the open JVM spec ( 
>>> <https://docs.oracle.com/javase/specs/jvms/se8/html/jvms-4.html#jvms-4.4-140>https://docs.oracle.com/javase/specs/jvms/se8/html/jvms-4.html#jvms-4.4-140
>>>  
>>> <https://docs.oracle.com/javase/specs/jvms/se8/html/jvms-4.html#jvms-4.4-140>).
>>> 
>>> Thanks
>>> -Konstantin
>>> 
>>> On 11/27/2015 07:48 PM, Konstantin Shefov wrote:
>>>> Coleen,
>>>> Thanks for review
>>>> 
>>>> On 11/24/2015 07:33 PM, Coleen Phillimore wrote:
>>>>> 
>>>>> I have a couple preliminary comments.  First, there are no tests added 
>>>>> with all this new functionality.  Tests should be added with the 
>>>>> functionality changeset, not promised afterward. 
>>>> I will add tests.
>>>>> Secondly, I do not like that JDK code knows the implementation details of 
>>>>> the JVM's constant pool tags.  These should be only for internal use.
>>>> The package "sun.reflect" is for internal use only, so it shouldn’t be a 
>>>> problem.
>>>>> My third comment is that I haven't looked carefully at this constant pool 
>>>>> implementation but it seems very unsafe if the class is redefined and 
>>>>> relies on an implementation detail in the JVM that can change.   I will 
>>>>> have more comments once I look more at the jvmti specification.
>>>>> 
>>>>> thanks,
>>>>> Coleen
>>>>> 
>>>>> On 11/24/15 9:48 AM, Konstantin Shefov wrote:
>>>>>> Hello
>>>>>> 
>>>>>> Please, review modified webrev.
>>>>>> 
>>>>>> I have added methods
>>>>>> getNameAndTypeRefIndexAt(int index) - to get name and type entry index 
>>>>>> from a method, field or indy entry index;
>>>>>> getClassRefIndexAt(int index) - to get class entry index from a method 
>>>>

Re: RFR [9] 8141615: Add new public methods to sun.reflect.ConstantPool

[Christian Thalinger]

> On Dec 11, 2015, at 1:54 AM, Konstantin Shefov <konstantin.she...@oracle.com> 
> wrote:
> 
> Hello
> 
> Please review the new version on the patch.
> 
> New webrev:
> Webrev hotspot: http://cr.openjdk.java.net/~kshefov/8141615/hotspot/webrev.02
> Webrev jdk: http://cr.openjdk.java.net/~kshefov/8141615/jdk/webrev.02 
> <http://cr.openjdk.java.net/~kshefov/8141615/jdk/webrev.02>

These newlines look ridiculous especially when it’s not even needed:

+  // Returns a class reference index for a method or a field.
+  public int  getClassRefIndexAt
+ (int index) { return getClassRefIndexAt0 
(constantPoolOop, index); }

Either fix the indent or just add them like regular methods should look like.

> 
> What has been changed:
> 1. Added tests for the new added methods.
> 2. Removed CP tag codes 100 - 105 from being passed to java and left only 
> codes from the open JVM spec 
> (https://docs.oracle.com/javase/specs/jvms/se8/html/jvms-4.html#jvms-4.4-140).
> 
> Thanks
> -Konstantin
> 
> On 11/27/2015 07:48 PM, Konstantin Shefov wrote:
>> Coleen,
>> Thanks for review
>> 
>> On 11/24/2015 07:33 PM, Coleen Phillimore wrote:
>>> 
>>> I have a couple preliminary comments.  First, there are no tests added with 
>>> all this new functionality.  Tests should be added with the functionality 
>>> changeset, not promised afterward. 
>> I will add tests.
>>> Secondly, I do not like that JDK code knows the implementation details of 
>>> the JVM's constant pool tags.  These should be only for internal use.
>> The package "sun.reflect" is for internal use only, so it shouldn’t be a 
>> problem.
>>> My third comment is that I haven't looked carefully at this constant pool 
>>> implementation but it seems very unsafe if the class is redefined and 
>>> relies on an implementation detail in the JVM that can change.   I will 
>>> have more comments once I look more at the jvmti specification.
>>> 
>>> thanks,
>>> Coleen
>>> 
>>> On 11/24/15 9:48 AM, Konstantin Shefov wrote:
>>>> Hello
>>>> 
>>>> Please, review modified webrev.
>>>> 
>>>> I have added methods
>>>> getNameAndTypeRefIndexAt(int index) - to get name and type entry index 
>>>> from a method, field or indy entry index;
>>>> getClassRefIndexAt(int index) - to get class entry index from a method or 
>>>> field entry index;
>>>> 
>>>> I removed previously added method
>>>> getInvokedynamicRefInfoAt(int index) - as it is no more needed now.
>>>> 
>>>> New webrev:
>>>> Webrev hotspot: 
>>>> http://cr.openjdk.java.net/~kshefov/8141615/hotspot/webrev.01
>>>> Webrev jdk: http://cr.openjdk.java.net/~kshefov/8141615/jdk/webrev.01
>>>> 
>>>> Thanks
>>>> -Konstantin
>>>> 
>>>> On 11/18/2015 02:11 PM, Konstantin Shefov wrote:
>>>>> Remi,
>>>>> 
>>>>> Thanks for reviewing. Your suggestion sounds sensibly.
>>>>> May be it also has sense to make a method 
>>>>> "getMethodRefNameAndTypeIndex(int index)" to acquire name-and-type entry 
>>>>> index for methods also.
>>>>> 
>>>>> -Konstantin
>>>>> 
>>>>> On 11/18/2015 12:04 AM, Remi Forax wrote:
>>>>>> Hi Konstantin,
>>>>>> Technically, getInvokedynamicRefInfoAt should be 
>>>>>> getNameAndTypeOfInvokedynamicRefInfoAt because it only extract the 
>>>>>> nameAndType value of the InvokeDynamicRef.
>>>>>> 
>>>>>> In term of API, I think it's better to decompose the API, i.e. to have a 
>>>>>> method
>>>>>>   int getInvokedynamicRefNameAndTypeIndex(int index)
>>>>>> that returns the nameAndType index and to reuse 
>>>>>> getNameAndTypeRefInfoAt(index) to get the corresponding array of Strings.
>>>>>> 
>>>>>> cheers,
>>>>>> Rémi
>>>>>> 
>>>>>> - Mail original -
>>>>>>> De: "Christian Thalinger" <christian.thalin...@oracle.com>
>>>>>>> À: "Konstantin Shefov" <konstantin.she...@oracle.com>
>>>>>>> Cc: "hotspot-dev developers" <hotspot-...@openjdk.java.net>, 
>>>>>>> core-libs-dev@openjdk.java.net
>>>>>>>

Re: RFR [9] 8141615: Add new public methods to sun.reflect.ConstantPool

[Christian Thalinger]

Kind of related but not really, I’ve filed a bug against ConstantPool to add 
JSR 292 “support”:

JDK-8077229: sun.reflect.ConstantPool should support class and method constant 
pools
https://bugs.openjdk.java.net/browse/JDK-8077229 
<https://bugs.openjdk.java.net/browse/JDK-8077229>

> On Nov 17, 2015, at 11:04 AM, Remi Forax <fo...@univ-mlv.fr> wrote:
> 
> Hi Konstantin,
> Technically, getInvokedynamicRefInfoAt should be 
> getNameAndTypeOfInvokedynamicRefInfoAt because it only extract the 
> nameAndType value of the InvokeDynamicRef.
> 
> In term of API, I think it's better to decompose the API, i.e. to have a 
> method
>  int getInvokedynamicRefNameAndTypeIndex(int index)
> that returns the nameAndType index and to reuse 
> getNameAndTypeRefInfoAt(index) to get the corresponding array of Strings. 
> 
> cheers,
> Rémi
> 
> - Mail original -
>> De: "Christian Thalinger" <christian.thalin...@oracle.com>
>> À: "Konstantin Shefov" <konstantin.she...@oracle.com>
>> Cc: "hotspot-dev developers" <hotspot-...@openjdk.java.net>, 
>> core-libs-dev@openjdk.java.net
>> Envoyé: Lundi 16 Novembre 2015 23:41:45
>> Objet: Re: RFR [9] 8141615: Add new public methods to
>> sun.reflect.ConstantPool
>> 
>> [CC'ing core-libs-dev]
>> 
>>> On Nov 13, 2015, at 4:55 AM, Konstantin Shefov
>>> <konstantin.she...@oracle.com> wrote:
>>> 
>>> Hello
>>> 
>>> Please review an enhancement to add three new methods to
>>> sun.reflect.ConstantPool class.
>>> The following methods are suggested:
>>> 
>>> public String[] getNameAndTypeRefInfoAt(int index) - returns string
>>> representation of name and type from a NameAndType constant pool entry
>>> with the specified index
>>> 
>>> public String[] getInvokedynamicRefInfoAt(int index) - returns string
>>> representation of name and type from an InvokeDynamic constant pool entry
>>> with the specified index
>>> 
>>> public Tag getTagAt(int index) - returns a Tag enum instance of a constant
>>> pool entry with the specified index
>>> 
>>> These three methods could be used for testing API working with constant
>>> pool, e.g. JVMCI.
>>> 
>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8141615
>>> Webrev hotspot:
>>> http://cr.openjdk.java.net/~kshefov/8141615/hotspot/webrev.00
>>> Webrev jdk: http://cr.openjdk.java.net/~kshefov/8141615/jdk/webrev.00
>>> 
>>> Thanks
>>> -Konstantin
>> 
>> 

Re: RFR [9] 8141615: Add new public methods to sun.reflect.ConstantPool

[Christian Thalinger]

[CC'ing core-libs-dev]

> On Nov 13, 2015, at 4:55 AM, Konstantin Shefov  
> wrote:
> 
> Hello
> 
> Please review an enhancement to add three new methods to 
> sun.reflect.ConstantPool class.
> The following methods are suggested:
> 
> public String[] getNameAndTypeRefInfoAt(int index) - returns string 
> representation of name and type from a NameAndType constant pool entry with 
> the specified index
> 
> public String[] getInvokedynamicRefInfoAt(int index) - returns string 
> representation of name and type from an InvokeDynamic constant pool entry 
> with the specified index
> 
> public Tag getTagAt(int index) - returns a Tag enum instance of a constant 
> pool entry with the specified index
> 
> These three methods could be used for testing API working with constant pool, 
> e.g. JVMCI.
> 
> JBS: https://bugs.openjdk.java.net/browse/JDK-8141615
> Webrev hotspot: http://cr.openjdk.java.net/~kshefov/8141615/hotspot/webrev.00
> Webrev jdk: http://cr.openjdk.java.net/~kshefov/8141615/jdk/webrev.00
> 
> Thanks
> -Konstantin

Re: JFokus VM Tech Day 2016

[Christian Thalinger]

> On Nov 11, 2015, at 3:37 PM, John Rose  wrote:
> 
> On Nov 11, 2015, at 12:35 AM, Marcus Lagergren  > wrote:
>> 
>> bare silicone magic
> 
> That extra E moves the venue from Santa Clara to Las Vegas.

LOL!  Too good.

> ___
> mlvm-dev mailing list
> mlvm-dev@openjdk.java.net
> http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

___
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

Re: RFR(XS): 8141416: "expr: syntax error" due to gcc -dumpversion excluding micro

[Christian Thalinger]

> On Nov 4, 2015, at 8:21 AM, Volker Simonis  wrote:
> 
> Hi,
> 
> can somebody please review and sponsor the following tiny build fix:
> 
> http://cr.openjdk.java.net/~simonis/webrevs/2015/8141416/
> https://bugs.openjdk.java.net/browse/JDK-8141416
> 
> Building hotspot on certain systems results in a series of:
> expr: syntax error
> expr: syntax error
> expr: syntax error
> expr: syntax error
> 
> It is caused by a peculiarity of the gcc version on Ubuntu where "gcc
> -dumpversion" doesn't print a micro-version:
> 
> Ubuntu:
> $ gcc -dumpversion
> 4.6
> 
> Any other Linux:
> $ gcc -dumpversion
> 4.8.3
> 
> This "feature" is tracked under
> https://bugs.launchpad.net/ubuntu/+source/gcc-4.8/+bug/1360404 and has
> been fixed for gcc 4.9 but won't be fixed for older versions of gcc.
> 
> In hotspot/make/linux/makefiles/gcc.make we parse the micro-version of
> gcc and use it in the following way:
> 
> CC_VER_MICRO := $(shell $(CC) -dumpversion | sed 's/egcs-//' | cut -d'.' -f3)

I’ve added this.  Thanks for fixing it.

> 
> ifeq ($(shell expr $(CC_VER_MAJOR) = 4 \& $(CC_VER_MINOR) = 1 \&
> $(CC_VER_MICRO) = 1), 1)
>  $(error "GCC $(CC_VER_MAJOR).$(CC_VER_MINOR).$(CC_VER_MICRO) not
> supported because of
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=27724;)
> endif
> 
> The shell expression results in a syntax error if $(CC_VER_MICRO)
> because it expands to something like "expr 4 = 4 & 3 = 1 & = 1"
> 
> Thank you and best regards,
> Volker

Re: RFR [XS] - 8139026 hotspot.script cannot handle command-line arguments with spaces

[Christian Thalinger]

> On Oct 25, 2015, at 10:34 PM, Ioi Lam  wrote:
> 
> Please review a very small fix:
> 
> http://cr.openjdk.java.net/~iklam/8139026-hotspot-script-arg-quoating/

Looks good.  I’m sure you asked, who is still using this script?

> 
> Bug: hotspot/make/hotspot.script cannot handle command-line arguments with 
> spaces
> 
> https://bugs.openjdk.java.net/browse/JDK-8139026
> 
> Summary of fix:
> 
>   The old script was adding $@ to a string like X="A B $@ C". Doing that would
>   lose the quotation on the arguments. This would cause JTREG to fail when 
> running
>   with Jigsaw modules (see bug report for details).
> 
>   The fix is to pass "$@" directly as arguments to all programs launched by
>   hotspot.script
> 
>   Note that the fix does not address the problem with DBX, but at least it's 
> no worse
>   than before.
> 
> Tests:
> 
>I have used the modified version for the past 2 weeks with GDB and JTREG 
> and found
>no issues.
> 
>Also, casual testing shows the quotation is retained:
> 
>$ hotspot 'a a'
>Error: Could not find or load main class a a
>$ hotspot "a' a"
>Error: Could not find or load main class a' a
>$ hotspot "a\"' a"
>Error: Could not find or load main class a"' a
> 
> Thanks
> - Ioi

Re: RFR (S): 8140091: remove VMStructs cast_uint64_t workaround for GCC 4.1.1 bug

[Christian Thalinger]

> On Oct 21, 2015, at 10:57 PM, Erik Joelsson <erik.joels...@oracle.com> wrote:
> 
> I think this is ok, but we should make sure to add a check in configure in 
> the hotspot build-infra project.

Yes, we should do that.  I was looking for something like this but couldn’t 
find any code so I went ahead and did it in the HotSpot Makefiles.

Do you want to add a check in configure with this bug or a separate one?

> 
> /Erik
> 
> On 2015-10-21 21:46, Christian Thalinger wrote:
>> https://bugs.openjdk.java.net/browse/JDK-8140091
>> http://cr.openjdk.java.net/~twisti/8140091/webrev/
>> 
>> There is a bit more context in JIRA but the bottom line is that we shouldn’t 
>> be building the JDK with GCC 4.1.1.  We have a workaround in just one place 
>> but that doesn’t mean the bug doesn’t show up in other places (especially 
>> new code).
>> 
>> I’m suggesting to remove the workaround and blacklist this particular GCC 
>> version.
> 

Re: 8139891: Prepare Unsafe for true encapsulation

[Christian Thalinger]

> On Oct 22, 2015, at 6:31 AM, Paul Sandoz  wrote:
> 
> Hi Chris,
> 
> This looks like a good first step. I am sure hotspot devs will have more 
> concrete review comments, but i like the way the method aliasing (copying the 
> same technique StrictMath -> Math) avoids any changes to the intrinsics, thus 
> the changes are much more localized.

Yes, me too.  I was worried about a lot of changes in compiler code so I’m 
really happy to see this is not the case.

> 
> It may be worth having some fast debugging output for the aliasing in case 
> internal refactoring messes things up.
> 
> Paul.
> 
>> On 22 Oct 2015, at 07:28, Chris Hegarty  wrote:
>> 
>> As part of the preparation for JEP 260 [1], Unsafe needs to be separable
>> from the base module [2], so it can be exported by a new, yet to be
>> defined, jdk.unsupported module, and have a separate evolution policy
>> that is not exposed. That is, the JDK needs an internal Unsafe that can
>> be evolved and added to in future releases, while maintaining the
>> existing Unsafe API that developers are using.
>> 
>> Many uses of Unsafe are for performance reasons. Any changes made
>> should preserve the current performance characteristics. To achieve this
>> the existing Unsafe intrinsic candidate methods should remain intrinsic
>> candidate methods. The VM can provide aliases for these intrinsics so
>> they are common to both the internal and sun.misc versions of Unsafe.
>> 
>> The JDK implementation will be updated to use the new internal version
>> of Unsafe.
>> 
>> For the purposes of making progress, I think this work can be split into
>> several steps:
>> 
>> 1) Create the new internal Unsafe class and provide intrinsic support
>>for both.
>> 2) Update existing, and possibly add new, tests to use the new
>>internal Unsafe. Some tests may need to be duplicated, or reworked,
>>to test both versions of Unsafe.
>> 3) Update the Unsafe usages in the JDK so that there is no longer any
>>dependency on sun.misc.Unsafe.
>> 
>> To this end I've put together a webrev containing the changes required
>> for step 1. It contains
>>  - the intrinsic aliasing,
>>  - the new internal Unsafe ( copy of sun.misc.Unsafe ),
>>  - reverts sun.misc.Unsafe to, almost, its 1.8 API,
>>  - minimal test and implementation updates, since there some usages
>>of unaligned access that is new in the 1.9.
>> 
>> http://cr.openjdk.java.net/~chegar/unsafe_phase1/
>> 
>> All JPRT hotspot and core libraries tests pass.
>> 
>> I have most of the work for steps 2 and 3 done, but some discussion and
>> clean up is required. It also increase the level of difficult to review
>> the changes in phase 1, which is mostly what I'd like to get agreement
>> on first.
>> 
>> -Chris.
>> 
>> [1] https://bugs.openjdk.java.net/browse/JDK-8132928
>> [2] https://bugs.openjdk.java.net/browse/JDK-8139891
> 

Re: RFR (XXS): 8139935: Bootcycle builds are broken on jdk9/hs due to JVMCI changes

[Christian Thalinger]

Thanks, Erik.

> On Oct 20, 2015, at 9:51 PM, Erik Joelsson <erik.joels...@oracle.com> wrote:
> 
> Looks good to me.
> 
> /Erik
> 
> On 2015-10-20 22:19, Christian Thalinger wrote:
>> https://bugs.openjdk.java.net/browse/JDK-8139935
>> 
>> The reason this “works” for a non-bootcycle build is because a JDK 9 javac 
>> won't enforce the “hidden packages” rules when running on a non-JDK 9 
>> platform.
>> 
>> diff -r a8a8604f890f make/gensrc/Gensrc-jdk.vm.ci.gmk
>> --- a/make/gensrc/Gensrc-jdk.vm.ci.gmk   Sat Oct 17 19:40:30 2015 -0400
>> +++ b/make/gensrc/Gensrc-jdk.vm.ci.gmk   Tue Oct 20 09:57:02 2015 -1000
>> @@ -77,6 +77,7 @@ PROCESSOR_PATH := $(call PathList, \
>>  $(MKDIR) -p $(@D)
>>  $(eval $(call ListPathsSafely,PROC_SRCS,$(@D)/_gensrc_proc_files))
>>  $(JAVA_SMALL) $(NEW_JAVAC) \
>> +-XDignore.symbol.file \
>>  -sourcepath $(SOURCEPATH) \
>>  -implicit:none \
>>  -proc:only \
>> 
> 

RFR (S): 8140091: remove VMStructs cast_uint64_t workaround for GCC 4.1.1 bug

[Christian Thalinger]

https://bugs.openjdk.java.net/browse/JDK-8140091
http://cr.openjdk.java.net/~twisti/8140091/webrev/

There is a bit more context in JIRA but the bottom line is that we shouldn’t be 
building the JDK with GCC 4.1.1.  We have a workaround in just one place but 
that doesn’t mean the bug doesn’t show up in other places (especially new code).

I’m suggesting to remove the workaround and blacklist this particular GCC 
version.

RFR (XXS): 8139935: Bootcycle builds are broken on jdk9/hs due to JVMCI changes

[Christian Thalinger]

https://bugs.openjdk.java.net/browse/JDK-8139935

The reason this “works” for a non-bootcycle build is because a JDK 9 javac 
won't enforce the “hidden packages” rules when running on a non-JDK 9 platform.

diff -r a8a8604f890f make/gensrc/Gensrc-jdk.vm.ci.gmk
--- a/make/gensrc/Gensrc-jdk.vm.ci.gmk  Sat Oct 17 19:40:30 2015 -0400
+++ b/make/gensrc/Gensrc-jdk.vm.ci.gmk  Tue Oct 20 09:57:02 2015 -1000
@@ -77,6 +77,7 @@ PROCESSOR_PATH := $(call PathList, \
$(MKDIR) -p $(@D)
$(eval $(call ListPathsSafely,PROC_SRCS,$(@D)/_gensrc_proc_files))
$(JAVA_SMALL) $(NEW_JAVAC) \
+   -XDignore.symbol.file \
-sourcepath $(SOURCEPATH) \
-implicit:none \
-proc:only \

Re: RFR: 8136556 - Add the ability to perform static builds of MacOSX x64 binaries

[Christian Thalinger]

> On Oct 19, 2015, at 8:09 AM, Bob Vandette  wrote:
> 
> Here’s the updated set of webrev’s that address two issues:
> 
> 1. Move jni_util.h out of jawt.h
> 2. Christians concern over using a single variable name for Makefile and 
> C/C++ logic.

Thanks.  Looks good to me.

> 
> http://cr.openjdk.java.net/~bobv/8136556/webrev.01 
> 
> http://cr.openjdk.java.net/~bobv/8136556/hotspot/webrev.01 
> 
> http://cr.openjdk.java.net/~bobv/8136556/jdk/webrev.01 
> 
> 
> Bob.
> 
>> On Oct 16, 2015, at 2:28 AM, Alan Bateman > > wrote:
>> 
>> On 15/10/2015 19:07, Bob Vandette wrote:
>>> Please review this JDK 9 enhancement which allows a completely static build 
>>> of the JDK for MacOSX x64 platforms.
>>> 
>>> https://bugs.openjdk.java.net/browse/JDK-8136556 
>>>  
>>> >> >
>>> 
>>> The change involves:
>>> 
>>> 1. Producing “.a” archives for each native libraries.
>>> 2. Ensuring that all symbols across the JDK native libraries are unique.
>>> 3. Changing the JNI_OnLoad and JNI_OnUnload (and the Agent equivalents) to 
>>> have the each library name appended per
>>>the JNI specification.
>>> 4. Modifications to the launcher and launcher Makefiles to allow them to be 
>>> linked with the java.base and jdk.jdwp.agent libraries
>>>and function.
>>> 
>>> http://cr.openjdk.java.net/~bobv/8136556/webrev.00/ 
>>>  
>>> >> >
>>> http://cr.openjdk.java.net/~bobv/8136556/hotspot/webrev.00/ 
>>>  
>>> >> >
>>> http://cr.openjdk.java.net/~bobv/8136556/jdk/webrev.00/ 
>>>  
>>> >> >
>>> 
>>> Note: This change does not link every possible static library with the 
>>> launchers.  It is currently limited to
>>> the java.base and jdk.jdwp.agent libraries in order to allow for the TCK 
>>> validation of the base module only.
>>> 
>> I've skimmed through the patches and the DEF_* macros look okay. The only 
>> one that doesn't look right is jawt.h/jawt.c. As jawt.h is shipped by the 
>> JDK then I think the include of jni_util.h needs to move from jawt.h to 
>> jawt.c.
>> 
>> If I read the changes correctly then not loading the 
>> JavaRuntimeSupport.framework on Mac means the locale might not be right, is 
>> that correct? Brent might remember this issue as we've often pondered the 
>> implications of this disappearing in an Mac update.
>> 
>> Will there be continuous or at least regular builds setup so that build 
>> breakages will be reported in a timely manner? It would be easy to change 
>> something that breaks the static library build.
>> 
>> -Alan
> 

Re: RFR: 8136556 - Add the ability to perform static builds of MacOSX x64 binaries

[Christian Thalinger]

> On Oct 19, 2015, at 8:09 AM, Bob Vandette  wrote:
> 
> Here’s the updated set of webrev’s that address two issues:
> 
> 1. Move jni_util.h out of jawt.h
> 2. Christians concern over using a single variable name for Makefile and 
> C/C++ logic.

Thanks.  Looks good to me.

> 
> http://cr.openjdk.java.net/~bobv/8136556/webrev.01 
> 
> http://cr.openjdk.java.net/~bobv/8136556/hotspot/webrev.01 
> 
> http://cr.openjdk.java.net/~bobv/8136556/jdk/webrev.01 
> 
> 
> Bob.
> 
>> On Oct 16, 2015, at 2:28 AM, Alan Bateman > > wrote:
>> 
>> On 15/10/2015 19:07, Bob Vandette wrote:
>>> Please review this JDK 9 enhancement which allows a completely static build 
>>> of the JDK for MacOSX x64 platforms.
>>> 
>>> https://bugs.openjdk.java.net/browse/JDK-8136556 
>>>  
>>> >> >
>>> 
>>> The change involves:
>>> 
>>> 1. Producing “.a” archives for each native libraries.
>>> 2. Ensuring that all symbols across the JDK native libraries are unique.
>>> 3. Changing the JNI_OnLoad and JNI_OnUnload (and the Agent equivalents) to 
>>> have the each library name appended per
>>>the JNI specification.
>>> 4. Modifications to the launcher and launcher Makefiles to allow them to be 
>>> linked with the java.base and jdk.jdwp.agent libraries
>>>and function.
>>> 
>>> http://cr.openjdk.java.net/~bobv/8136556/webrev.00/ 
>>>  
>>> >> >
>>> http://cr.openjdk.java.net/~bobv/8136556/hotspot/webrev.00/ 
>>>  
>>> >> >
>>> http://cr.openjdk.java.net/~bobv/8136556/jdk/webrev.00/ 
>>>  
>>> >> >
>>> 
>>> Note: This change does not link every possible static library with the 
>>> launchers.  It is currently limited to
>>> the java.base and jdk.jdwp.agent libraries in order to allow for the TCK 
>>> validation of the base module only.
>>> 
>> I've skimmed through the patches and the DEF_* macros look okay. The only 
>> one that doesn't look right is jawt.h/jawt.c. As jawt.h is shipped by the 
>> JDK then I think the include of jni_util.h needs to move from jawt.h to 
>> jawt.c.
>> 
>> If I read the changes correctly then not loading the 
>> JavaRuntimeSupport.framework on Mac means the locale might not be right, is 
>> that correct? Brent might remember this issue as we've often pondered the 
>> implications of this disappearing in an Mac update.
>> 
>> Will there be continuous or at least regular builds setup so that build 
>> breakages will be reported in a timely manner? It would be easy to change 
>> something that breaks the static library build.
>> 
>> -Alan
> 

Re: RFR: 8136556 - Add the ability to perform static builds of MacOSX x64 binaries

[Christian Thalinger]

Copy-pasting the comment I made earlier (internally):

>> make/bsd/makefiles/gcc.make:
>> 
>> + ifeq ($(BUILD_STATIC),true)
>> + CXXFLAGS += -DSTATIC_BUILD
>> + CFLAGS += -DSTATIC_BUILD
>> 
>> Can we use the same name everywhere?
> 
> We were trying to differentiate Makefile options from compile time 
> conditionals.
> In one case it’s set to true and the other it’s defined.
> 
> Are there no other cases where this is done?

I’m not sure but looking at make/excludeSrc.make all of them use the same name.

> On Oct 15, 2015, at 8:10 AM, Bob Vandette  wrote:
> 
> Please review this JDK 9 enhancement which allows a completely static build 
> of the JDK for MacOSX x64 platforms.
> 
> https://bugs.openjdk.java.net/browse/JDK-8136556 
> 
> 
> The change involves:
> 
> 1. Producing “.a” archives for each native libraries.
> 2. Ensuring that all symbols across the JDK native libraries are unique.
> 3. Changing the JNI_OnLoad and JNI_OnUnload (and the Agent equivalents) to 
> have the each library name appended per
>   the JNI specification.
> 4. Modifications to the launcher and launcher Makefiles to allow them to be 
> linked with the java.base and jdk.jdwp.agent libraries
>   and function.
> 
> http://cr.openjdk.java.net/~bobv/8136556/webrev.00/ 
> 
> http://cr.openjdk.java.net/~bobv/8136556/hotspot/webrev.00/ 
> 
> http://cr.openjdk.java.net/~bobv/8136556/jdk/webrev.00/ 
> 
> 
> Note: This change does not link every possible static library with the 
> launchers.  It is currently limited to
> the java.base and jdk.jdwp.agent libraries in order to allow for the TCK 
> validation of the base module only.
> 
> 
> Bob.

Re: RFR: JDK-8139657: Incremental build of jdk.vm.ci-gensrc creates repeated entries in services file

[Christian Thalinger]

Looks good.

Since you are a JDK 9 Reviewer, will you push this fix?

> On Oct 15, 2015, at 2:39 AM, Erik Joelsson <erik.joels...@oracle.com> wrote:
> 
> Hello,
> 
> Sorry for not noticing this earlier. Here is a fix for the problem with 
> repeating lines in the services file.
> 
> Bug: https://bugs.openjdk.java.net/browse/JDK-8139657
> Patch:
> diff -r 9ab5571ccea8 make/gensrc/Gensrc-jdk.vm.ci.gmk
> --- a/make/gensrc/Gensrc-jdk.vm.ci.gmk
> +++ b/make/gensrc/Gensrc-jdk.vm.ci.gmk
> @@ -108,7 +108,11 @@
> ($(CD) $(GENSRC_DIR)/META-INF/jvmci.providers && \
> for i in $$($(LS)); do \
>   c=$$($(CAT) $$i | $(TR) -d '\n\r'); \
> -  $(ECHO) $$i >> $(GENSRC_DIR)/META-INF/services/$$c; \
> +  $(ECHO) $$i >> $(GENSRC_DIR)/META-INF/services/$$c.tmp; \
> +done)
> +($(CD) $(GENSRC_DIR)/META-INF/services && \
> +for i in $$($(LS) *.tmp); do \
> +  $(MV) $$i $${i%.tmp}; \
> done)
> $(TOUCH) $@
> 
> /Erik
> 
> On 2015-10-08 04:42, Christian Thalinger wrote:
>>> On Oct 5, 2015, at 2:47 AM, Magnus Ihse Bursie 
>>> <magnus.ihse.bur...@oracle.com> wrote:
>>> 
>>> On 2015-09-29 03:12, Christian Thalinger wrote:
>>>>> On Sep 27, 2015, at 11:25 PM, Magnus Ihse Bursie 
>>>>> <magnus.ihse.bur...@oracle.com> wrote:
>>>>> 
>>>>> On 2015-09-25 22:00, Christian Thalinger wrote:
>>>>>> Btw. we found a bug in creating the OptionDescriptors files; we get 
>>>>>> duplicate entries:
>>>>>> 
>>>>>> $ cat 
>>>>>> build/macosx-x86_64-normal-server-release/jdk/modules/java.base/META-INF/services/jdk.internal.jvmci.options.OptionDescriptors
>>>>>> jdk.internal.jvmci.compiler.Compiler_OptionDescriptors
>>>>>> jdk.internal.jvmci.hotspot.HotSpotConstantReflectionProvider_OptionDescriptors
>>>>>> jdk.internal.jvmci.hotspot.HotSpotResolvedJavaFieldImpl_OptionDescriptors
>>>>>> jdk.internal.jvmci.hotspot.HotSpotResolvedJavaMethodImpl_OptionDescriptors
>>>>>> jdk.internal.jvmci.compiler.Compiler_OptionDescriptors
>>>>>> jdk.internal.jvmci.hotspot.HotSpotConstantReflectionProvider_OptionDescriptors
>>>>>> jdk.internal.jvmci.hotspot.HotSpotResolvedJavaFieldImpl_OptionDescriptors
>>>>>> jdk.internal.jvmci.hotspot.HotSpotResolvedJavaMethodImpl_OptionDescriptors
>>>>>> …
>>>>>> 
>>>>>> Would this be the right fix?
>>>>>> 
>>>>>> diff -r db1a815d2f6c make/gensrc/Gensrc-java.base.gmk
>>>>>> --- a/make/gensrc/Gensrc-java.base.gmkThu Sep 24 15:35:49 2015 -1000
>>>>>> +++ b/make/gensrc/Gensrc-java.base.gmkFri Sep 25 18:18:35 2015 +0200
>>>>>> @@ -94,6 +94,7 @@
>>>>>>$(GENSRC_DIR)/_gensrc_proc_done
>>>>>> $(MKDIR) -p $(@D)
>>>>>> ($(CD) $(GENSRC_DIR)/META-INF/jvmci.options && \
>>>>>> +$(RM) -f $@; \
>>>>>>for i in $$(ls); do \
>>>>>>  echo $${i}_OptionDescriptors >> $@; \
>>>>>>done)
>>>>>> 
>>>>> That seems like a reasonable fix, yes.
>>>> Thanks, but… (see below)
>>>> 
>>>>>> And I see the same behavior for HotSpotJVMCIBackendFactory:
>>>>>> 
>>>>>> $ cat 
>>>>>> build/macosx-x86_64-normal-server-release/jdk/modules/java.base/META-INF/services/jdk.internal.jvmci.hotspot.HotSpotJVMCIBackendFactory
>>>>>> jdk.internal.jvmci.hotspot.amd64.AMD64HotSpotJVMCIBackendFactory
>>>>>> jdk.internal.jvmci.hotspot.sparc.SPARCHotSpotJVMCIBackendFactory
>>>>>> jdk.internal.jvmci.hotspot.amd64.AMD64HotSpotJVMCIBackendFactory
>>>>>> jdk.internal.jvmci.hotspot.sparc.SPARCHotSpotJVMCIBackendFactory
>>>>>> …
>>>>>> 
>>>>>> So I think a similar fix needs to be applied there.
>>>> …I’ve look at the code that creates this file and it isn’t obvious to me 
>>>> how to fix it.  Any good ideas?
>>> Try this:
>>> ($(CD) $(GENSRC_DIR)/META-INF/jvmci.providers && \
>>> for i in $$($(LS)); do \
>>>   c=$$($(CAT) $$i | $(TR) -d '\n\r'); \
>>> +  $(RM) $(GENSRC_DIR)/META-INF/services/$$c; \
>>>   $(ECHO) $$i >> $(GENSRC_DIR)/META-INF/services/$$c; \
>>> done)
>>> $(TOUCH) $@
>>> 
>>> I have not tested it but it should work.
>> No, this won’t work.  Both implementations of HotSpotJVMCIBackendFactory 
>> (AMD64HotSpotJVMCIBackendFactory and SPARCHotSpotJVMCIBackendFactory) 
>> contain the same service file name:
>> 
>> jdk.internal.jvmci.hotspot.HotSpotJVMCIBackendFactory
>> 
>> since we need to collect all available factories in one service.
>> 
>>> /Magnus
> 

Re: RFR: 8136556 - Add the ability to perform static builds of MacOSX x64 binaries

[Christian Thalinger]

Copy-pasting the comment I made earlier (internally):

>> make/bsd/makefiles/gcc.make:
>> 
>> + ifeq ($(BUILD_STATIC),true)
>> + CXXFLAGS += -DSTATIC_BUILD
>> + CFLAGS += -DSTATIC_BUILD
>> 
>> Can we use the same name everywhere?
> 
> We were trying to differentiate Makefile options from compile time 
> conditionals.
> In one case it’s set to true and the other it’s defined.
> 
> Are there no other cases where this is done?

I’m not sure but looking at make/excludeSrc.make all of them use the same name.

> On Oct 15, 2015, at 8:10 AM, Bob Vandette  wrote:
> 
> Please review this JDK 9 enhancement which allows a completely static build 
> of the JDK for MacOSX x64 platforms.
> 
> https://bugs.openjdk.java.net/browse/JDK-8136556 
> 
> 
> The change involves:
> 
> 1. Producing “.a” archives for each native libraries.
> 2. Ensuring that all symbols across the JDK native libraries are unique.
> 3. Changing the JNI_OnLoad and JNI_OnUnload (and the Agent equivalents) to 
> have the each library name appended per
>   the JNI specification.
> 4. Modifications to the launcher and launcher Makefiles to allow them to be 
> linked with the java.base and jdk.jdwp.agent libraries
>   and function.
> 
> http://cr.openjdk.java.net/~bobv/8136556/webrev.00/ 
> 
> http://cr.openjdk.java.net/~bobv/8136556/hotspot/webrev.00/ 
> 
> http://cr.openjdk.java.net/~bobv/8136556/jdk/webrev.00/ 
> 
> 
> Note: This change does not link every possible static library with the 
> launchers.  It is currently limited to
> the java.base and jdk.jdwp.agent libraries in order to allow for the TCK 
> validation of the base module only.
> 
> 
> Bob.

Re: CLANG special case

[Christian Thalinger]

I’ve assigned the bug to me and will verify…

> On Oct 6, 2015, at 9:23 AM, Jim Laskey (Oracle)  
> wrote:
> 
> Since we are will to live with the de-optimization now, we should just remove 
> the condition until proven otherwise. 
> 
> 
>> On Oct 6, 2015, at 4:17 PM, Staffan Larsen  wrote:
>> 
>> When we upgraded to clang 6.3, I verified that the problem still existed. 
>> See: https://bugs.openjdk.java.net/browse/JDK-8077364 
>>  which has pointers to the 
>> two tests that fail without the workaround.
>> 
>> /Staffan
>> 
>>> On 6 okt 2015, at 17:38, Phil Race >> > wrote:
>>> 
>>> Ideally hotspot would review this, not build.
>>> so it would be helpful if hotspot found an engineer to own the bug :-
>>> https://bugs.openjdk.java.net/browse/JDK-8138820 
>>> 
>>> So far as I know this is not tracked under any other bug id.
>>> 
>>> -phil.
>>> 
>>> On 10/06/2015 05:30 AM, Jim Laskey (Oracle) wrote:
 I’ve updated to El Capitan and, of course, builds fail, and, of course, I 
 modify hotspot/make/bsd/makefiles/gcc.make one more time and…   I think 
 this conditional clause should be removed at the very least (commenting to 
 indicate needs investigation), or someone should research and see which 
 version of clang fixes the issues associate with the patch.  Since it’s 
 likely that no one has the cycles, please remove the condition.
 
 Cheers,
 
 — Jim
 
 
 
 diff -r a02911828e48 make/bsd/makefiles/gcc.make
 --- a/make/bsd/makefiles/gcc.make  Wed Sep 30 07:41:36 2015 -0700
 +++ b/make/bsd/makefiles/gcc.make  Tue Oct 06 09:22:50 2015 -0300
 @@ -313,21 +313,13 @@
   # Work around some compiler bugs.
 ifeq ($(USE_CLANG), true)
 -  # Clang <= 6.1
 -  ifeq ($(shell expr \
 -  $(CC_VER_MAJOR) \< 6 \| \
 -  \( $(CC_VER_MAJOR) = 6 \& $(CC_VER_MINOR) \<= 1 \) \
 -), 1)
 -OPT_CFLAGS/loopTransform.o += $(OPT_CFLAGS/NOOPT)
 -OPT_CFLAGS/unsafe.o += -O1
 -  else
 -$(error "Update compiler workarounds for Clang 
 $(CC_VER_MAJOR).$(CC_VER_MINOR)")
 -  endif
 +  OPT_CFLAGS/loopTransform.o += $(OPT_CFLAGS/NOOPT)
 +  OPT_CFLAGS/unsafe.o += -O1
 else
   # 6835796. Problem in GCC 4.3.0 with mulnode.o optimized compilation.
   ifeq ($(shell expr $(CC_VER_MAJOR) = 4 \& $(CC_VER_MINOR) = 3), 1)
 OPT_CFLAGS/mulnode.o += $(OPT_CFLAGS/NOOPT)
 -  endif
 +  endif
 endif
   # Flags for generating make dependency flags.
 
>>> 
>> 
> 

Re: RFR (XXL): JEP 243: Java-Level JVM Compiler Interface

[Christian Thalinger]

> On Sep 17, 2015, at 10:20 PM, Volker Simonis <volker.simo...@gmail.com> wrote:
> 
> For the top-level change I always get a strange error when importing it:
> 
> patch failed, unable to continue (try -v)
> patch failed, rejects left in working dir
> errors during apply, please fix and refresh 8062493_jvmci_top_0911.v1.patch
> 
> It is because of the strange path syntax of the last hunk in the patch file:
> 
> --- old/./modules.xml 2015-09-16 15:11:14.0 -0700
> +++ new/./modules.xml 2015-09-16 15:11:14.0 -0700
> @@ -254,6 +254,14 @@
>   jdk.jfr
> 
> 
> +  jdk.internal.jvmci.hotspot
> +  jdk.jfr
> +
> +
> +  jdk.internal.jvmci.hotspot.events
> +  jdk.jfr
> +
> +
>   sun.misc
>   java.corba
>   java.desktop
> 
> 
> Notice the strange syntax "old/./modules.xml" and "new/./modules.xml".
> If I remove the redundant './' from the path, everything works fine.
> For some reason only the diffs for modules.xml has this strange path
> syntax in the patch.

That’s odd.  Vladimir created the webrevs.  Maybe he did something different 
with the top-level one.

> 
> Regards,
> Volker
> 
> 
> On Thu, Sep 17, 2015 at 10:32 PM, Christian Thalinger
> <christian.thalin...@oracle.com> wrote:
>> Since there are no objections I’m going to push this…
>> 
>> http://hg.openjdk.java.net/graal/graal-jvmci-9/hotspot/rev/6a6766a8cbbf
>> 
>>> On Sep 16, 2015, at 3:05 PM, Christian Thalinger 
>>> <christian.thalin...@oracle.com> wrote:
>>> 
>>> I would like to add this change:
>>> 
>>> diff -r 2134e08cc132 src/share/vm/utilities/vmError.cpp
>>> --- a/src/share/vm/utilities/vmError.cpp  Wed Sep 16 14:28:33 2015 -1000
>>> +++ b/src/share/vm/utilities/vmError.cpp  Wed Sep 16 15:04:02 2015 -1000
>>> @@ -517,6 +517,10 @@ void VMError::report(outputStream* st) {
>>>   Abstract_VM_Version::vm_release(),
>>>   Abstract_VM_Version::vm_info_string(),
>>>   TieredCompilation ? ", tiered" : "",
>>> +#if INCLUDE_JVMCI
>>> +   EnableJVMCI ? ", jvmci" : "",
>>> +   UseJVMCICompiler ? ", jvmci compiler" : "",
>>> +#endif
>>>   UseCompressedOops ? ", compressed oops" : "",
>>>       gc_mode(),
>>>   Abstract_VM_Version::vm_platform_string()
>>> 
>>> It would be useful to see in the crash logs if this experimental feature 
>>> was turned on.
>>> 
>>>> On Sep 16, 2015, at 12:43 PM, Vladimir Kozlov <vladimir.koz...@oracle.com> 
>>>> wrote:
>>>> 
>>>> I updated top and hotspot webrev with latest (build) changes.
>>>> 
>>>> Vladimir
>>>> 
>>>> On 9/16/15 2:28 PM, Christian Thalinger wrote:
>>>>> 
>>>>>> On Sep 16, 2015, at 6:52 AM, Christian Thalinger 
>>>>>> <christian.thalin...@oracle.com> wrote:
>>>>>> 
>>>>>> 
>>>>>>> On Sep 16, 2015, at 2:57 AM, Magnus Ihse Bursie 
>>>>>>> <magnus.ihse.bur...@oracle.com> wrote:
>>>>>>> 
>>>>>>> On 2015-09-14 09:24, Christian Thalinger wrote:
>>>>>>>> The JEP itself can be found here:
>>>>>>>> 
>>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8062493 
>>>>>>>> <https://bugs.openjdk.java.net/browse/JDK-8062493>
>>>>>>>> 
>>>>>>>> Here are the webrevs:
>>>>>>>> 
>>>>>>>> http://cr.openjdk.java.net/~kvn/JVMCI/webrev.top/ 
>>>>>>>> <http://cr.openjdk.java.net/~kvn/JVMCI/webrev.top/>
>>>>>>>> http://cr.openjdk.java.net/~kvn/JVMCI/webrev.hotspot/ 
>>>>>>>> <http://cr.openjdk.java.net/~kvn/JVMCI/webrev.hotspot/>
>>>>>>>> 
>>>>>>>> The change has already undergone a few iterations of internal review 
>>>>>>>> by different people of different teams.  Most comments and suggestions 
>>>>>>>> were handled accordingly.  Although there is one open item we agreed 
>>>>>>>> we will address after the integration of JEP 243 and that work is 
>>>>>>>> captured in RFE:

Re: RFR (XXL): JEP 243: Java-Level JVM Compiler Interface

[Christian Thalinger]

Since there are no objections I’m going to push this…

http://hg.openjdk.java.net/graal/graal-jvmci-9/hotspot/rev/6a6766a8cbbf

> On Sep 16, 2015, at 3:05 PM, Christian Thalinger 
> <christian.thalin...@oracle.com> wrote:
> 
> I would like to add this change:
> 
> diff -r 2134e08cc132 src/share/vm/utilities/vmError.cpp
> --- a/src/share/vm/utilities/vmError.cpp  Wed Sep 16 14:28:33 2015 -1000
> +++ b/src/share/vm/utilities/vmError.cpp  Wed Sep 16 15:04:02 2015 -1000
> @@ -517,6 +517,10 @@ void VMError::report(outputStream* st) {
>Abstract_VM_Version::vm_release(),
>Abstract_VM_Version::vm_info_string(),
>TieredCompilation ? ", tiered" : "",
> +#if INCLUDE_JVMCI
> +   EnableJVMCI ? ", jvmci" : "",
> +   UseJVMCICompiler ? ", jvmci compiler" : "",
> +#endif
>UseCompressedOops ? ", compressed oops" : "",
>gc_mode(),
>Abstract_VM_Version::vm_platform_string()
> 
> It would be useful to see in the crash logs if this experimental feature was 
> turned on.
> 
>> On Sep 16, 2015, at 12:43 PM, Vladimir Kozlov <vladimir.koz...@oracle.com> 
>> wrote:
>> 
>> I updated top and hotspot webrev with latest (build) changes.
>> 
>> Vladimir
>> 
>> On 9/16/15 2:28 PM, Christian Thalinger wrote:
>>> 
>>>> On Sep 16, 2015, at 6:52 AM, Christian Thalinger 
>>>> <christian.thalin...@oracle.com> wrote:
>>>> 
>>>> 
>>>>> On Sep 16, 2015, at 2:57 AM, Magnus Ihse Bursie 
>>>>> <magnus.ihse.bur...@oracle.com> wrote:
>>>>> 
>>>>> On 2015-09-14 09:24, Christian Thalinger wrote:
>>>>>> The JEP itself can be found here:
>>>>>> 
>>>>>> https://bugs.openjdk.java.net/browse/JDK-8062493 
>>>>>> <https://bugs.openjdk.java.net/browse/JDK-8062493>
>>>>>> 
>>>>>> Here are the webrevs:
>>>>>> 
>>>>>> http://cr.openjdk.java.net/~kvn/JVMCI/webrev.top/ 
>>>>>> <http://cr.openjdk.java.net/~kvn/JVMCI/webrev.top/>
>>>>>> http://cr.openjdk.java.net/~kvn/JVMCI/webrev.hotspot/ 
>>>>>> <http://cr.openjdk.java.net/~kvn/JVMCI/webrev.hotspot/>
>>>>>> 
>>>>>> The change has already undergone a few iterations of internal review by 
>>>>>> different people of different teams.  Most comments and suggestions were 
>>>>>> handled accordingly.  Although there is one open item we agreed we will 
>>>>>> address after the integration of JEP 243 and that work is captured in 
>>>>>> RFE:
>>>>>> 
>>>>>> https://bugs.openjdk.java.net/browse/JDK-8134994 
>>>>>> <https://bugs.openjdk.java.net/browse/JDK-8134994>
>>>>>> 
>>>>>> SQE is still working on the tests and all test tasks can be seen as 
>>>>>> sub-tasks of the JEP.
>>>>>> 
>>>>>> The integration will happen under the bug number:
>>>>>> 
>>>>>> https://bugs.openjdk.java.net/browse/JDK-8136421 
>>>>>> <https://bugs.openjdk.java.net/browse/JDK-8136421>
>>>>>> 
>>>>> Hi Christian,
>>>>> 
>>>>> (Adding build-dev since this review includes some major build changes.)
>>>>> 
>>>>> The makefile changes looks good in general to me. I have not reviewed the 
>>>>> source code changes.
>>>>> 
>>>>> Some comments:
>>>>> 
>>>>> * modules.xml:
>>>>> Make sure you get sign-off from a Jigsaw developer for modifying this 
>>>>> file.
>>>> 
>>>> I’ve asked Alan to take a look.
>>>> 
>>>>> 
>>>>> * hotspot/make/linux/makefiles/gcc.make:
>>>>> Seems unfortunate to have to disable a new warning 
>>>>> (undefined-bool-conversion) for newly incorporated code. Is it not 
>>>>> possible to fix the code emitting this warning instead?
>>>> 
>>>> We had this question before.  It’s not obvious because you can’t see it in 
>>>> the regular diff views but this is under:
>>>> 
>>>> ifeq ($(USE_CLANG), true)
>>>> 
>>>>> 
>>>>&g

Re: RFR (XXL): JEP 243: Java-Level JVM Compiler Interface

[Christian Thalinger]

> On Sep 16, 2015, at 2:57 AM, Magnus Ihse Bursie 
> <magnus.ihse.bur...@oracle.com> wrote:
> 
> On 2015-09-14 09:24, Christian Thalinger wrote:
>> The JEP itself can be found here:
>> 
>> https://bugs.openjdk.java.net/browse/JDK-8062493 
>> <https://bugs.openjdk.java.net/browse/JDK-8062493>
>> 
>> Here are the webrevs:
>> 
>> http://cr.openjdk.java.net/~kvn/JVMCI/webrev.top/ 
>> <http://cr.openjdk.java.net/~kvn/JVMCI/webrev.top/>
>> http://cr.openjdk.java.net/~kvn/JVMCI/webrev.hotspot/ 
>> <http://cr.openjdk.java.net/~kvn/JVMCI/webrev.hotspot/>
>> 
>> The change has already undergone a few iterations of internal review by 
>> different people of different teams.  Most comments and suggestions were 
>> handled accordingly.  Although there is one open item we agreed we will 
>> address after the integration of JEP 243 and that work is captured in RFE:
>> 
>> https://bugs.openjdk.java.net/browse/JDK-8134994 
>> <https://bugs.openjdk.java.net/browse/JDK-8134994>
>> 
>> SQE is still working on the tests and all test tasks can be seen as 
>> sub-tasks of the JEP.
>> 
>> The integration will happen under the bug number:
>> 
>> https://bugs.openjdk.java.net/browse/JDK-8136421 
>> <https://bugs.openjdk.java.net/browse/JDK-8136421>
>> 
> Hi Christian,
> 
> (Adding build-dev since this review includes some major build changes.)
> 
> The makefile changes looks good in general to me. I have not reviewed the 
> source code changes.
> 
> Some comments:
> 
> * modules.xml:
> Make sure you get sign-off from a Jigsaw developer for modifying this file.

I’ve asked Alan to take a look.

> 
> * hotspot/make/linux/makefiles/gcc.make:
> Seems unfortunate to have to disable a new warning 
> (undefined-bool-conversion) for newly incorporated code. Is it not possible 
> to fix the code emitting this warning instead?

We had this question before.  It’s not obvious because you can’t see it in the 
regular diff views but this is under:

ifeq ($(USE_CLANG), true)

> 
> * make/common/MakeBase.gmk and hotspot/make/gensrc/Gensrc-java.base.gmk:
> I see a coming merge conflict. In jdk9/dev, there is now a new function that 
> performs the same function as CreatePath, but it's named PathList. (It's a 
> bit unfortunate that this code snippet has bounced around as patches without 
> a definite name.) I assume you are going to push this through a hotspot 
> forest. If the PathList patch reaches the hotspot repo before this, please 
> remove the CreatePath from MakeBase, and change the calls to CreatePath to 
> PathList instead. (I could only find such calls in 
> hotspot/make/gensrc/Gensrc-java.base.gmk). If this patch goes in before that, 
> we'll need to give a heads-up to the integrator about this conflict.

Thanks for the heads up.

> 
> Another potential coming merge conflict relates to ListPathsSafely in 
> Gensrc-java.base.gmk. There is currenlty a review out from Erik which 
> modifies the API for ListPathsSafely. If/when it goes in, the call to 
> ListPathsSafely in Gensrc-java.base.gmk will need to be modified (Erik can 
> give advice on how). Depending on timing, this too might hit the integrator 
> rather than your push.

Ok, thanks.

> 
> /Magnus

Re: RFR (XXL): JEP 243: Java-Level JVM Compiler Interface

[Christian Thalinger]

> On Sep 16, 2015, at 10:11 AM, Magnus Ihse Bursie 
> <magnus.ihse.bur...@oracle.com> wrote:
> 
> On 2015-09-16 18:52, Christian Thalinger wrote:
>>> On Sep 16, 2015, at 2:57 AM, Magnus Ihse Bursie 
>>> <magnus.ihse.bur...@oracle.com> wrote:
>>> 
>>> On 2015-09-14 09:24, Christian Thalinger wrote:
>>>> The JEP itself can be found here:
>>>> 
>>>> https://bugs.openjdk.java.net/browse/JDK-8062493 
>>>> <https://bugs.openjdk.java.net/browse/JDK-8062493>
>>>> 
>>>> Here are the webrevs:
>>>> 
>>>> http://cr.openjdk.java.net/~kvn/JVMCI/webrev.top/ 
>>>> <http://cr.openjdk.java.net/~kvn/JVMCI/webrev.top/>
>>>> http://cr.openjdk.java.net/~kvn/JVMCI/webrev.hotspot/ 
>>>> <http://cr.openjdk.java.net/~kvn/JVMCI/webrev.hotspot/>
>>>> 
>>>> The change has already undergone a few iterations of internal review by 
>>>> different people of different teams.  Most comments and suggestions were 
>>>> handled accordingly. Although there is one open item we agreed we will 
>>>> address after the integration of JEP 243 and that work is captured in RFE:
>>>> 
>>>> https://bugs.openjdk.java.net/browse/JDK-8134994 
>>>> <https://bugs.openjdk.java.net/browse/JDK-8134994>
>>>> 
>>>> SQE is still working on the tests and all test tasks can be seen as 
>>>> sub-tasks of the JEP.
>>>> 
>>>> The integration will happen under the bug number:
>>>> 
>>>> https://bugs.openjdk.java.net/browse/JDK-8136421 
>>>> <https://bugs.openjdk.java.net/browse/JDK-8136421>
>>>> 
>>> Hi Christian,
>>> 
>>> (Adding build-dev since this review includes some major build changes.)
>>> 
>>> The makefile changes looks good in general to me. I have not reviewed the 
>>> source code changes.
>>> 
>>> Some comments:
>>> 
>>> * modules.xml:
>>> Make sure you get sign-off from a Jigsaw developer for modifying this file.
>> I’ve asked Alan to take a look.
>> 
>>> * hotspot/make/linux/makefiles/gcc.make:
>>> Seems unfortunate to have to disable a new warning 
>>> (undefined-bool-conversion) for newly incorporated code. Is it not possible 
>>> to fix the code emitting this warning instead?
>> We had this question before.  It’s not obvious because you can’t see it in 
>> the regular diff views but this is under:
>> 
>> ifeq ($(USE_CLANG), true)
> 
> I'm not sure I understand why that's relevant..? Isn't it just as important 
> to try to submit warning-free code when compiling with clang as with any 
> other compiler? Or is clang just being anal about the code in question?

I don’t have a Clang compiler at hand but Clang is anal about everything.  Do 
you want that suppression to be removed?

> 
> /Magnus
> 
>> 
>>> * make/common/MakeBase.gmk and hotspot/make/gensrc/Gensrc-java.base.gmk:
>>> I see a coming merge conflict. In jdk9/dev, there is now a new function 
>>> that performs the same function as CreatePath, but it's named PathList. 
>>> (It's a bit unfortunate that this code snippet has bounced around as 
>>> patches without a definite name.) I assume you are going to push this 
>>> through a hotspot forest. If the PathList patch reaches the hotspot repo 
>>> before this, please remove the CreatePath from MakeBase, and change the 
>>> calls to CreatePath to PathList instead. (I could only find such calls in 
>>> hotspot/make/gensrc/Gensrc-java.base.gmk). If this patch goes in before 
>>> that, we'll need to give a heads-up to the integrator about this conflict.
>> Thanks for the heads up.
>> 
>>> Another potential coming merge conflict relates to ListPathsSafely in 
>>> Gensrc-java.base.gmk. There is currenlty a review out from Erik which 
>>> modifies the API for ListPathsSafely. If/when it goes in, the call to 
>>> ListPathsSafely in Gensrc-java.base.gmk will need to be modified (Erik can 
>>> give advice on how). Depending on timing, this too might hit the integrator 
>>> rather than your push.
>> Ok, thanks.
>> 
>>> /Magnus

Re: RFR (XXL): JEP 243: Java-Level JVM Compiler Interface

[Christian Thalinger]

> On Sep 16, 2015, at 6:52 AM, Christian Thalinger 
> <christian.thalin...@oracle.com> wrote:
> 
> 
>> On Sep 16, 2015, at 2:57 AM, Magnus Ihse Bursie 
>> <magnus.ihse.bur...@oracle.com> wrote:
>> 
>> On 2015-09-14 09:24, Christian Thalinger wrote:
>>> The JEP itself can be found here:
>>> 
>>> https://bugs.openjdk.java.net/browse/JDK-8062493 
>>> <https://bugs.openjdk.java.net/browse/JDK-8062493>
>>> 
>>> Here are the webrevs:
>>> 
>>> http://cr.openjdk.java.net/~kvn/JVMCI/webrev.top/ 
>>> <http://cr.openjdk.java.net/~kvn/JVMCI/webrev.top/>
>>> http://cr.openjdk.java.net/~kvn/JVMCI/webrev.hotspot/ 
>>> <http://cr.openjdk.java.net/~kvn/JVMCI/webrev.hotspot/>
>>> 
>>> The change has already undergone a few iterations of internal review by 
>>> different people of different teams.  Most comments and suggestions were 
>>> handled accordingly.  Although there is one open item we agreed we will 
>>> address after the integration of JEP 243 and that work is captured in RFE:
>>> 
>>> https://bugs.openjdk.java.net/browse/JDK-8134994 
>>> <https://bugs.openjdk.java.net/browse/JDK-8134994>
>>> 
>>> SQE is still working on the tests and all test tasks can be seen as 
>>> sub-tasks of the JEP.
>>> 
>>> The integration will happen under the bug number:
>>> 
>>> https://bugs.openjdk.java.net/browse/JDK-8136421 
>>> <https://bugs.openjdk.java.net/browse/JDK-8136421>
>>> 
>> Hi Christian,
>> 
>> (Adding build-dev since this review includes some major build changes.)
>> 
>> The makefile changes looks good in general to me. I have not reviewed the 
>> source code changes.
>> 
>> Some comments:
>> 
>> * modules.xml:
>> Make sure you get sign-off from a Jigsaw developer for modifying this file.
> 
> I’ve asked Alan to take a look.
> 
>> 
>> * hotspot/make/linux/makefiles/gcc.make:
>> Seems unfortunate to have to disable a new warning 
>> (undefined-bool-conversion) for newly incorporated code. Is it not possible 
>> to fix the code emitting this warning instead?
> 
> We had this question before.  It’s not obvious because you can’t see it in 
> the regular diff views but this is under:
> 
> ifeq ($(USE_CLANG), true)
> 
>> 
>> * make/common/MakeBase.gmk and hotspot/make/gensrc/Gensrc-java.base.gmk:
>> I see a coming merge conflict. In jdk9/dev, there is now a new function that 
>> performs the same function as CreatePath, but it's named PathList. (It's a 
>> bit unfortunate that this code snippet has bounced around as patches without 
>> a definite name.) I assume you are going to push this through a hotspot 
>> forest. If the PathList patch reaches the hotspot repo before this, please 
>> remove the CreatePath from MakeBase, and change the calls to CreatePath to 
>> PathList instead. (I could only find such calls in 
>> hotspot/make/gensrc/Gensrc-java.base.gmk). If this patch goes in before 
>> that, we'll need to give a heads-up to the integrator about this conflict.
> 
> Thanks for the heads up.

Erik sent me a patch to avoid merge conflicts.  I’ve integrated two changesets:

http://hg.openjdk.java.net/graal/graal-jvmci-9/rev/ddedccc5c0ab 
<http://hg.openjdk.java.net/graal/graal-jvmci-9/rev/ddedccc5c0ab>
http://hg.openjdk.java.net/graal/graal-jvmci-9/hotspot/rev/fee6b89199c9 
<http://hg.openjdk.java.net/graal/graal-jvmci-9/hotspot/rev/fee6b89199c9>

> 
>> 
>> Another potential coming merge conflict relates to ListPathsSafely in 
>> Gensrc-java.base.gmk. There is currenlty a review out from Erik which 
>> modifies the API for ListPathsSafely. If/when it goes in, the call to 
>> ListPathsSafely in Gensrc-java.base.gmk will need to be modified (Erik can 
>> give advice on how). Depending on timing, this too might hit the integrator 
>> rather than your push.
> 
> Ok, thanks.
> 
>> 
>> /Magnus
> 

Re: RFR (XXL): JEP 243: Java-Level JVM Compiler Interface

[Christian Thalinger]

I would like to add this change:

diff -r 2134e08cc132 src/share/vm/utilities/vmError.cpp
--- a/src/share/vm/utilities/vmError.cppWed Sep 16 14:28:33 2015 -1000
+++ b/src/share/vm/utilities/vmError.cppWed Sep 16 15:04:02 2015 -1000
@@ -517,6 +517,10 @@ void VMError::report(outputStream* st) {
Abstract_VM_Version::vm_release(),
Abstract_VM_Version::vm_info_string(),
TieredCompilation ? ", tiered" : "",
+#if INCLUDE_JVMCI
+   EnableJVMCI ? ", jvmci" : "",
+   UseJVMCICompiler ? ", jvmci compiler" : "",
+#endif
UseCompressedOops ? ", compressed oops" : "",
gc_mode(),
Abstract_VM_Version::vm_platform_string()

It would be useful to see in the crash logs if this experimental feature was 
turned on.

> On Sep 16, 2015, at 12:43 PM, Vladimir Kozlov <vladimir.koz...@oracle.com> 
> wrote:
> 
> I updated top and hotspot webrev with latest (build) changes.
> 
> Vladimir
> 
> On 9/16/15 2:28 PM, Christian Thalinger wrote:
>> 
>>> On Sep 16, 2015, at 6:52 AM, Christian Thalinger 
>>> <christian.thalin...@oracle.com> wrote:
>>> 
>>> 
>>>> On Sep 16, 2015, at 2:57 AM, Magnus Ihse Bursie 
>>>> <magnus.ihse.bur...@oracle.com> wrote:
>>>> 
>>>> On 2015-09-14 09:24, Christian Thalinger wrote:
>>>>> The JEP itself can be found here:
>>>>> 
>>>>> https://bugs.openjdk.java.net/browse/JDK-8062493 
>>>>> <https://bugs.openjdk.java.net/browse/JDK-8062493>
>>>>> 
>>>>> Here are the webrevs:
>>>>> 
>>>>> http://cr.openjdk.java.net/~kvn/JVMCI/webrev.top/ 
>>>>> <http://cr.openjdk.java.net/~kvn/JVMCI/webrev.top/>
>>>>> http://cr.openjdk.java.net/~kvn/JVMCI/webrev.hotspot/ 
>>>>> <http://cr.openjdk.java.net/~kvn/JVMCI/webrev.hotspot/>
>>>>> 
>>>>> The change has already undergone a few iterations of internal review by 
>>>>> different people of different teams.  Most comments and suggestions were 
>>>>> handled accordingly.  Although there is one open item we agreed we will 
>>>>> address after the integration of JEP 243 and that work is captured in RFE:
>>>>> 
>>>>> https://bugs.openjdk.java.net/browse/JDK-8134994 
>>>>> <https://bugs.openjdk.java.net/browse/JDK-8134994>
>>>>> 
>>>>> SQE is still working on the tests and all test tasks can be seen as 
>>>>> sub-tasks of the JEP.
>>>>> 
>>>>> The integration will happen under the bug number:
>>>>> 
>>>>> https://bugs.openjdk.java.net/browse/JDK-8136421 
>>>>> <https://bugs.openjdk.java.net/browse/JDK-8136421>
>>>>> 
>>>> Hi Christian,
>>>> 
>>>> (Adding build-dev since this review includes some major build changes.)
>>>> 
>>>> The makefile changes looks good in general to me. I have not reviewed the 
>>>> source code changes.
>>>> 
>>>> Some comments:
>>>> 
>>>> * modules.xml:
>>>> Make sure you get sign-off from a Jigsaw developer for modifying this file.
>>> 
>>> I’ve asked Alan to take a look.
>>> 
>>>> 
>>>> * hotspot/make/linux/makefiles/gcc.make:
>>>> Seems unfortunate to have to disable a new warning 
>>>> (undefined-bool-conversion) for newly incorporated code. Is it not 
>>>> possible to fix the code emitting this warning instead?
>>> 
>>> We had this question before.  It’s not obvious because you can’t see it in 
>>> the regular diff views but this is under:
>>> 
>>> ifeq ($(USE_CLANG), true)
>>> 
>>>> 
>>>> * make/common/MakeBase.gmk and hotspot/make/gensrc/Gensrc-java.base.gmk:
>>>> I see a coming merge conflict. In jdk9/dev, there is now a new function 
>>>> that performs the same function as CreatePath, but it's named PathList. 
>>>> (It's a bit unfortunate that this code snippet has bounced around as 
>>>> patches without a definite name.) I assume you are going to push this 
>>>> through a hotspot forest. If the PathList patch reaches the hotspot repo 
>>>> before this, please remove the CreatePath from MakeBase, and change the 
>>>> calls to CreatePath to PathList instead. (I 

Re: JEP 248: Make G1 the Default Garbage Collector

[Christian Thalinger]

 On Apr 30, 2015, at 7:29 AM, Uwe Schindler uschind...@apache.org wrote:
 
 Hi Kirk, hi Mark,
 
 the Lucene/Solr/Elasticsearch people still recommend to their users to not 
 use G1GC, although for this type of application (full text search with the 
 requirement for very low response times and no pauses) is a good candidate 
 for G1GC. On the other hand, heap sizes for typical Lucene applications 
 should not be too high, because most of the processing is done on memory 
 mapped files off-heap. So heaps should be at most 1/4 of the physical RAM 
 available, because Lucene relies on the fact that the index files reside in 
 file system cache (too large heaps are contra-productive here).
 
 See also our recommendations for Apache Solr and Elasticsearch:
 http://wiki.apache.org/solr/ShawnHeisey#GC_Tuning_for_Solr
 http://www.elastic.co/guide/en/elasticsearch/guide/current/_don_8217_t_touch_these_settings.html
 
 Currently Lucene's indexing sometimes caused serious data corruption with 
 G1GC - leading to data loss, which was mainly caused by some bugs around G1GC 
 and its use of additional memory barriers and the very close interaction with 
 Hotspot, that seemed to broke some optimizations. We had (only in combination 
 with G1GC during our test suites) simple assert statements *sometimes* 
 failing that should never fail unless there is a bug in the JVM.

In fact there was a bug with asserts triggering when they shouldn’t:

https://bugs.openjdk.java.net/browse/JDK-8006960 
https://bugs.openjdk.java.net/browse/JDK-8006960

 
 We are aware that Java 8u40 declared G1GC as production ready, so we are 
 still looking at failures in our extensive testing infrastructure. Indeed, I 
 have no seen any G1GC related problems recently, but that is not necessarily 
 a sign for correctness.
 
 Uwe
 
 P.S.: It was nice to meet you last week on JAX!
 
 -
 Uwe Schindler
 uschind...@apache.org 
 ASF Member, Apache Lucene PMC / Committer
 Bremen, Germany
 http://lucene.apache.org/
 
 -Original Message-
 From: hotspot-dev [mailto:hotspot-dev-boun...@openjdk.java.net] On
 Behalf Of Kirk Pepperdine
 Sent: Wednesday, April 29, 2015 9:11 AM
 To: hotspot-...@openjdk.java.net Source Developers
 Subject: Re: JEP 248: Make G1 the Default Garbage Collector
 
 Hi all,
 
 Is the G1 ready for this? I see many people moving to G1 but also I’m not
 sure that we’ve got the tunable correct. I’ve been sorting through a number
 of recent tuning engagements and my  conclusion is that I would like the
 collector to be aggressive about collecting tenured regions at the beginning
 of a JVM’s life time but then become less aggressive over time. The reason is
 the residual waste that I see left behind because certain regions never hit
 the threshold needed to be included in the CSET. But, on aggregate, the
 number of regions in this state does start to retain a significant about of 
 dead
 data. The only way to see the effects is to run regular Full GCs.. which of
 course you don’t really want to do. However, the problem seems to settle
 down a wee bit over time which is why I was thinking that being aggressive
 about what is collected in the early stages of a JVMs life should lead to 
 better
 packing and hence less waste.
 
 Note, I don’t really care about the memory waste, only it’s effect on cycle
 frequencies and pause times.
 
 Sorry but I don’t have anything formal about this as I (and I believe many
 others) are still sorting out what to make of the G1 in prod. Generally the
 overall results are good but sometimes it’s not that way up front and how to
 improve things is sometimes challenging.
 
 On a side note, the move to Tiered in 8 has also caused a bit of grief.
 Metaspace has caused a bit of grief and even parallelStream, which works,
 has come with some interesting side effect. Everyone has been so enamored
 with Lambdas (rightfully so) that the other stuff has been completely
 forgotten and some of it has surprised people. I guess I’ll be submitting a 
 talk
 for J1 on some of the field experience I’ve had with the other stuff.
 
 Regards,
 Kirk
 
 
 On Apr 28, 2015, at 11:02 PM, mark.reinh...@oracle.com wrote:
 
 New JEP Candidate: http://openjdk.java.net/jeps/248
 
 - Mark
 

Re: ClassValue perf?

[Christian Thalinger]

 On Apr 24, 2015, at 2:17 PM, John Rose john.r.r...@oracle.com wrote:
 
 On Apr 24, 2015, at 5:38 AM, Charles Oliver Nutter head...@headius.com 
 wrote:
 
 Hey folks!
 
 I'm wondering how the performance of ClassValue looks on recent
 OpenJDK 7 and 8 builds. JRuby 9000 will be Java 7+ only, so this is
 one place I'd like to simplify our code a bit.
 
 I could measure myself, but I'm guessing some of you have already done
 a lot of exploration or have benchmarks handy. So, what say you?
 
 I'm listening too.  We don't have any special optimizations for CVs,
 and I'm hoping the generic code is a good-enough start.

A while ago (wow; it’s more than a year already) I was working on:

[#JDK-8031043] ClassValue's backing map should have a smaller initial size - 
Java Bug System https://bugs.openjdk.java.net/browse/JDK-8031043

and we had a conversation about it:

http://mail.openjdk.java.net/pipermail/mlvm-dev/2014-January/005597.html 
http://mail.openjdk.java.net/pipermail/mlvm-dev/2014-January/005597.html

It’s not about performance directly but it’s about memory usage and maybe the 
one-value-per-class optimization John suggests is in fact a performance 
improvement.  Someone should pick this one up.

 — John
 ___
 mlvm-dev mailing list
 mlvm-dev@openjdk.java.net
 http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

___
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

Re: RFR - 8072450: 9-dev build failed on elinux-i586 and rlinux-i586

[Christian Thalinger]

 On Feb 4, 2015, at 3:31 AM, Daniel Fuchs daniel.fu...@oracle.com wrote:
 
 Hi,
 
 Please find below a fix for:
 
 8072450: 9-dev build failed on elinux-i586 and rlinux-i586
 
 My fix for JDK-8068730 which was integrated from hs-rt into jdk9-dev
 yesterday is causing a build failure in the jdk9/dev nightly on two platforms.
 It appears that these platforms have a slightly older version
 of the compiler - which chokes on jvm.cpp with the following error:
 
 hotspot/src/share/vm/prims/jvm.cpp:307: error: integer constant is too large 
 for ���long��� type
 
 Adding a LL suffix should solve the issue:
 
 ##
 
 diff --git a/src/share/vm/prims/jvm.cpp b/src/share/vm/prims/jvm.cpp
 --- a/src/share/vm/prims/jvm.cpp
 +++ b/src/share/vm/prims/jvm.cpp
 @@ -304,7 +304,7 @@
 // java.lang.System, but we choose to keep it here so that it stays next
 // to JVM_CurrentTimeMillis and JVM_NanoTime
 
 -const jlong MAX_DIFF_SECS = 0x01;   //  2^32
 +const jlong MAX_DIFF_SECS = 0x01LL; //  2^32

Don’t use LL directly; we have a compiler-dependent macro for this:

CONST64

 const jlong MIN_DIFF_SECS = -MAX_DIFF_SECS; // -2^32
 
 JVM_LEAF(jlong, JVM_GetNanoTimeAdjustment(JNIEnv *env, jclass ignored, jlong 
 offset_secs))
 
 ##
 
 ( or if you prefer here is a webrev:
  http://cr.openjdk.java.net/~dfuchs/webrev_8072450/webrev.00 )
 
 best regards,
 
 -- daniel
 

Re: RFR(S): 8071999: SA's buildreplayjars fail with exception

[Christian Thalinger]

Since you’re saying it only fixes it partially should we file a followup bug 
and maybe leave a comment behind?

 On Feb 4, 2015, at 7:04 AM, Roland Westrelin roland.westre...@oracle.com 
 wrote:
 
 
 buildreplayjars sometimes fail with an exception and I think that's
 caused by the lack of support for default methods when the SA dumps
 classes. This change fixes it at least partially. It may not be the
 cleanest way to support default methods but it already proved useful
 when investigating a compiler crash so unless someone has a better fix
 I would like to push it.
 
 http://cr.openjdk.java.net/~roland/8071999/webrev.00/
 
 Roland.

Re: RFR - 8072450: 9-dev build failed on elinux-i586 and rlinux-i586

[Christian Thalinger]

 On Feb 4, 2015, at 9:44 AM, Daniel Fuchs daniel.fu...@oracle.com wrote:
 
 On 04/02/15 17:21, Christian Thalinger wrote:
 
 -const jlong MAX_DIFF_SECS = 0x01;   //  2^32
 +const jlong MAX_DIFF_SECS = 0x01LL; //  2^32
 
 Don’t use LL directly; we have a compiler-dependent macro for this:
 
 CONST64
 
 
 Hi Christian,
 
 I have pushed the change as it was reviewed by David, Coleen,  Staffan.
 
 I have logged https://bugs.openjdk.java.net/browse/JDK-8072482
 (assigned to myself) as a followup bug.

That’s fine, thanks.  Maybe you can fix a couple other places where LL-usage 
crept in too.

 
 best regards,
 
 -- daniel

Re: Invokedynamic and recursive method call

[Christian Thalinger]

Trying to remember compiler implementation details this sounds reasonable and 
is a bug (or an enhancement, actually ;-).  Can someone file a bug?

 On Jan 7, 2015, at 10:07 AM, Charles Oliver Nutter head...@headius.com 
 wrote:
 
 This could explain performance regressions we've seen on the
 performance of heavily-recursive algorithms. I'll try to get an
 assembly dump for fib in JRuby later today.
 
 - Charlie
 
 On Wed, Jan 7, 2015 at 10:13 AM, Remi Forax fo...@univ-mlv.fr wrote:
 
 On 01/07/2015 10:43 AM, Marcus Lagergren wrote:
 
 Remi, I tried to reproduce your problem with jdk9 b44. It runs decently
 fast.
 
 
 yes, nashorn is fast enough but it can be faster if the JIT was not doing
 something stupid.
 
 When the VM inline fibo, because fibo is recursive, the recursive call is
 inlined only once,
 so the call at depth=2 can not be inlined but should be a classical direct
 call.
 
 But if fibo is called through an invokedynamic, instead of emitting a direct
 call to fibo,
 the JIT generates a code that push the method handle on stack and execute it
 like if the metod handle was not constant
 (the method handle is constant because the call at depth=1 is inlined !).
 
 When did it start to regress?
 
 
 jdk7u40, i believe.
 
 I've created a jar containing some handwritten bytecodes with no dependency
 to reproduce the issue easily:
  https://github.com/forax/vmboiler/blob/master/test7/fibo7.jar
 
 [forax@localhost test7]$ time /usr/jdk/jdk1.9.0/bin/java -cp fibo7.jar
 FiboSample
 1836311903
 
 real0m6.653s
 user0m6.729s
 sys0m0.019s
 [forax@localhost test7]$ time /usr/jdk/jdk1.8.0_25/bin/java -cp fibo7.jar
 FiboSample
 1836311903
 
 real0m6.572s
 user0m6.591s
 sys0m0.019s
 [forax@localhost test7]$ time /usr/jdk/jdk1.7.0_71/bin/java -cp fibo7.jar
 FiboSample
 1836311903
 
 real0m6.373s
 user0m6.396s
 sys0m0.016s
 [forax@localhost test7]$ time /usr/jdk/jdk1.7.0_25/bin/java -cp fibo7.jar
 FiboSample
 1836311903
 
 real0m4.847s
 user0m4.832s
 sys0m0.019s
 
 as you can see, it was faster with a JDK before jdk7u40.
 
 
 Regards
 Marcus
 
 
 cheers,
 Rémi
 
 
 
 On 30 Dec 2014, at 20:48, Remi Forax fo...@univ-mlv.fr wrote:
 
 Hi guys,
 I've found a bug in the interaction between the lambda form and inlining
 algorithm,
 basically if the inlining heuristic bailout because the method is
 recursive and already inlined once,
 instead to emit a code to do a direct call, it revert to do call to
 linkStatic with the method
 as MemberName.
 
 I think it's a regression because before the introduction of lambda
 forms,
 I'm pretty sure that the JIT was emitting a direct call.
 
 Step to reproduce with nashorn, run this JavaScript code
 function fibo(n) {
  return (n  2)? 1: fibo(n - 1) + fibo(n - 2)
 }
 
 print(fibo(45))
 
 like this:
  /usr/jdk/jdk1.9.0/bin/jjs -J-XX:+UnlockDiagnosticVMOptions
 -J-XX:+PrintAssembly fibo.js  log.txt
 
 look for a method 'fibo' from the tail of the log, you will find
 something like this:
 
  0x7f97e4b4743f: mov$0x76d08f770,%r8   ;   {oop(a
 'java/lang/invoke/MemberName' = {method} {0x7f97dcff8e40} 'fibo'
 '(Ljdk/nashorn/internal/runtime/ScriptFunction;Ljava/lang/Object;I)I' in
 'jdk/nashorn/internal/scripts/Script$Recompilation$2$fibo')}
  0x7f97e4b47449: xchg   %ax,%ax
  0x7f97e4b4744b: callq  0x7f97dd0446e0
 
 I hope this can be fixed. My demonstration that I can have fibo written
 with a dynamic language
 that run as fast as written in Java doesn't work anymore :(
 
 cheers,
 Rémi
 
 ___
 mlvm-dev mailing list
 mlvm-dev@openjdk.java.net
 http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev
 
 ___
 mlvm-dev mailing list
 mlvm-dev@openjdk.java.net
 http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev
 
 
 ___
 mlvm-dev mailing list
 mlvm-dev@openjdk.java.net
 http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev
 ___
 mlvm-dev mailing list
 mlvm-dev@openjdk.java.net
 http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

___
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

Re: Invokedynamic and recursive method call

[Christian Thalinger]

 On Jan 29, 2015, at 4:48 PM, John Rose john.r.r...@oracle.com wrote:
 
 On Jan 7, 2015, at 8:13 AM, Remi Forax fo...@univ-mlv.fr 
 mailto:fo...@univ-mlv.fr wrote:
 
 But if fibo is called through an invokedynamic, instead of emitting a direct 
 call to fibo,
 the JIT generates a code that push the method handle on stack and execute it
 like if the metod handle was not constant
 (the method handle is constant because the call at depth=1 is inlined !).
 
 Invocation of non-constant MH's had a performance regression with the 
 LF-based implementation.
 As of JDK-8069591 they should be no slower and sometimes faster than the old 
 implementation.

Maybe but what Remi is saying that the MH is constant and we could emit a 
direct call.

 — John
 
 ___
 mlvm-dev mailing list
 mlvm-dev@openjdk.java.net
 http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

___
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

Re: RFR: 8059625 - JEP-JDK-8043304: Test task: DTrace- tests for segmented codecache feature

[Christian Thalinger]

  40 public static int[] getAvailableCompilationLevels() {
  41 if (System.getProperty(java.vm.info).startsWith(interpreted )) 
{
  42 return new int[0];
  43 }

Wouldn’t it be better to check the UseCompiler flag instead?

 On Dec 19, 2014, at 11:03 AM, Dmitrij Pochepko dmitrij.poche...@oracle.com 
 wrote:
 
 Hi all,
 
 Please review changes for https://bugs.openjdk.java.net/browse/JDK-8059625 -  
 JEP-JDK-8043304: Test task: DTrace- tests for segmented codecache feature
 
 Description: this fix introduce dtrace test, which verify that different 
 combinations of available compile levels(and, in case compile levels allows 
 it, different code heaps as result)  doesn't affect callstack shown by 
 dtrace. There is a control class SegmentedCodeCacheDtraceTest.java and class 
 for running via dtrace SegmentedCodeCacheDtraceTestWorker.java. A dtrace d 
 script is also present (SegmentedCodeCacheDtraceTestScript.d). A control 
 class is using DtraceRunner.java to run dtrace and then analyzing results 
 using class SegmentedCodeCacheDtraceResultsAnalyzer with 
 DtraceResultsAnalyzer interface.
 There is also a small class CompilerUtils.java created for usefull common 
 code.
 
 webrev: http://cr.openjdk.java.net/~iignatyev/dpochepk/8059625/webrev.00/
 
 Additional note: Please note that this path assumes that fix for JDK-8066440 
 - Various changes in testlibrary for JDK-8059613 is also applied.
 
 Thanks,
 Dmitrij

Re: Adding Microbenchmarks to the JDK forest/trees (JEP-230)

[Christian Thalinger]

 On Dec 2, 2014, at 2:40 PM, Jonathan Gibbons jonathan.gibb...@oracle.com 
 wrote:
 
 Staffan,
 
 That seems to put it on the low end for reasonably being its own repo, if you 
 wanted that, at least, as indicated by the numbers.

Do we really want more repositories?

 
 Here's the file counts for where we are now
 
 corba 1192
 hotspot 4761
 jaxp 2883
 jaxws 3748
 jdk 22776
 langtools 6785
 
 -- Jon
 
 On 12/02/2014 02:27 PM, Staffan Friberg wrote:
 Hi Jon,
 
 As part of the initial set of benchmarks we hope to add as part of this JEP 
 I'm guessing it will be around 200-300 files. This would grow overtime, but 
 I believe we won't see tens of thousands of files, it is more likely it will 
 be something like a 1000 files.
 
 //Staffan
 
 On 12/02/2014 02:14 PM, Jonathan Gibbons wrote:
 Staffan,
 
 I would also ask how many files are eventually likely to be involved.
 
 If it's tens of files up to low hundreds, then a top level dir makes sense.
 
 If it's tens of thousands of files, then a separate repo makes more sense.
 
 -- Jon
 
 
 On 12/02/2014 02:08 PM, Staffan Friberg wrote:
 Hi Chris,
 
 Agree, there is no major reason this needs to be a new repository, as I 
 mentioned in the 3 options below it would work well without it. The main 
 thing I want to achieve is that the benchmarks are located on the top 
 level. The suite will contain benchmarks for all parts of the JDK so 
 having it in either jdk or hotspot doesn't feel like it makes sense. If 
 people agree on having it as folder in the top level JDK repository I'm 
 perfectly fine with that.
 
 As for building it will most likely not be of the general build process 
 for building the JDK (do not want to increase the compilation time for 
 anyone not requiring the benchmark suite). It would have its own target 
 'build-microbenchmark' which would depend on 'exploded-image', but not the 
 reverse.
 
 //Staffan
 
 On 12/02/2014 01:23 PM, Chris Hegarty wrote:
 Staffan,
 
 Having all the benchmarks located in a single place makes sense to me, 
 but this doesn’t necessarily mean that they need their own repository, in 
 the forest.
 
 If I can build, run, and test ( usual development cycle ) without any 
 dependency on these benchmarks, or their infrastructure, essential 
 working with a partial forest ( without the ‘benchmark’ repository ), 
 then I can see the possible value in having a separate repository ( so I 
 can skip cloning and updating it ). But, I’m not sure if that is a 
 reasonable justification for a new repository, as it is probably at odds 
 with your goals, or maybe not?
 
 -Chris
 
 On 2 Dec 2014, at 19:53, Staffan Friberg staffan.frib...@oracle.com 
 wrote:
 
 Hi,
 
 (Adding the jdk9-dev list to increase the visibility of the discussion)
 
 With the multiple sub-repository commit mechanism improved I believe 
 this might be less of an issue. JPRT can push JDK and HS changes at 
 together and the same functionality should be possible to use for this 
 as well. I wonder if the test issue earlier was that it was a completely 
 separate repository outside of the JDK forest, and less of an issue when 
 being part of the same forest as the JDK source code. Perhaps someone 
 from SQE can chime?
 
 Otherwise the main reason for having a separate sub-repository on the 
 top level is making it easier to find what benchmarks are available and 
 have a single place to add new once avoid any risk of name duplication. 
 JMH is superb in filtering during execution during runtime so running 
 just a single test or a group of tests is very straight forward and the 
 recommended way, rather than having multiple benchmark JARs. It also 
 makes the build process easier as the building can be done using a 
 single Makefile and a single benchmark JAR (actually two, one for JDK 8 
 compatible tests and one for JDK 9) that can be picked up by automatic 
 performance testing.
 
 Cheers,
 Staffan
 
 On 12/02/2014 06:48 AM, roger riggs wrote:
 Hi Staffan,
 
 An earlier issue was keeping tests in sync with the code under test, 
 hence
 the use of test directories within each repository.
 I think a structure in which the benchmarks for some function and the 
 function
 itself are in the same repository that is easier to understand and 
 maintain.
 
 $.02, Roger
 
 
 On 12/1/2014 7:08 PM, Staffan Friberg wrote:
 Hi,
 
 Hopefully this is the right list for this discussion.
 
 As part of adding Microbenchmarks to the OpenJDK source tree, I'm 
 trying to understand how we best would add the benchmark sources to 
 the existing OpenJDK tree structure.
 
 Since the microbenchmark suite will cover all parts of the JDK, 
 covering HotSpot, JDK libraries and Nashorn, it would be preferred to 
 add the microbenchmark directory as a new top level directory. 
 Something similar to the following structure. Having benchmark as 
 the top-level directory would allow us to later add different types of 
 benchmarks without colliding with the microbenchmark suite.
 
 

Re: RFR: AARCH64: Top-level JDK changes

[Christian Thalinger]

 On Nov 19, 2014, at 12:56 AM, Erik Joelsson erik.joels...@oracle.com wrote:
 
 
 On 2014-11-18 19:09, Christian Thalinger wrote:
 On Nov 18, 2014, at 7:36 AM, Magnus Ihse Bursie 
 magnus.ihse.bur...@oracle.com wrote:
 
 On 2014-11-18 01:59, Christian Thalinger wrote:
 On Nov 17, 2014, at 4:11 AM, Magnus Ihse Bursie 
 magnus.ihse.bur...@oracle.com wrote:
 
 On 2014-11-14 21:44, Dean Long wrote:
 The distribution exception is there exactly since anyone should be able 
 to distribute the files with their configure script. That does not mean 
 that you are allowed to edit it, though.
 What if we require Autoconf to be installed on the host?  Does that 
 solve any problems?
 No, unfortunately not.
 Why not?
 Autoconf picks up these files automatically from the build-aux directory. 
 That's also the reason we need to rename the original files and provide 
 wrappers with the same name, since we can't even redirect that 
 functionality to a file with another name.
 So do I understand you correctly that the files we need are automatically 
 copied into the workspace but since we want to use our own, old versions we 
 renamed them and use these instead?
 No, I will try to clarify.
 
 Autoconf is a tool that takes one (or more) input files (written in m4 macro 
 language) and generates a shell script, often named configure. This shell 
 script is what you would typically run to configure your project. Autoconf 
 defines an API of m4 macros specifically for configure scripts which is 
 basically what makes it useful. Most of these macros are expanded into the 
 generated configure script.
 
 However, for reasons unknown to us, some of the more complex functionality 
 has been split out into separate shell script library files. These library 
 files, often referred to as build-aux are supposed to be distributed with 
 the project source code, along with the generated configure script. We 
 distribute them in common/autoconf/build-aux. These files can be found in the 
 source distribution of autoconf or by downloading from the official scm repo 
 for them. They are not part of the binary distribution of autoconf on my 
 Ubuntu system at least.

Well, that’s because config.guess and config.sub are part of automake:

http://git.savannah.gnu.org/gitweb/?p=automake.git;a=tree;f=lib;hb=HEAD 
http://git.savannah.gnu.org/gitweb/?p=automake.git;a=tree;f=lib;hb=HEAD

and installed e.g. in this directory on Solaris:

$ ls /usr/share/automake-1.10/
acinstall*  ansi2knr.1  Automake/  config-ml.in   config.sub*  depcomp* 
INSTALL  mdate-sh*  mkinstalldirs*  symlink-tree*  ylwrap*
am/ ansi2knr.c  compile*   config.guess*  COPYING  elisp-comp*  
install-sh*  missing*   py-compile* texinfo.tex

$ automake --add-missing

makes a copy of these files, if necessary:

configure.ac:3: the top level
configure.ac:3: installing `./config.sub'
configure.ac:3: installing `./config.guess'

$ ls -l config.*
lrwxrwxrwx 1 cthaling staff 37 2014-11-19 16:26 config.guess - 
/usr/share/automake-1.10/config.guess*
lrwxrwxrwx 1 cthaling staff 35 2014-11-19 16:26 config.sub - 
/usr/share/automake-1.10/config.sub*

 For this reason, it wouldn't help requiring autoconf to be installed as that 
 wouldn't provide the files.
 
 For non GPL projects to be able to distribute the files in build-aux, they 
 come with a special exception to GPL, which basically allows them to be 
 freely distributed as long as they are part of a configure script. This 
 exception does not seem to give any exception for deriving work from them.
 
 /Erik

Re: RFR: AARCH64: Top-level JDK changes

[Christian Thalinger]

 On Nov 18, 2014, at 7:36 AM, Magnus Ihse Bursie 
 magnus.ihse.bur...@oracle.com wrote:
 
 On 2014-11-18 01:59, Christian Thalinger wrote:
 On Nov 17, 2014, at 4:11 AM, Magnus Ihse Bursie 
 magnus.ihse.bur...@oracle.com wrote:
 
 On 2014-11-14 21:44, Dean Long wrote:
 The distribution exception is there exactly since anyone should be able 
 to distribute the files with their configure script. That does not mean 
 that you are allowed to edit it, though.
 What if we require Autoconf to be installed on the host?  Does that solve 
 any problems?
 No, unfortunately not.
 Why not?
 
 Autoconf picks up these files automatically from the build-aux directory. 
 That's also the reason we need to rename the original files and provide 
 wrappers with the same name, since we can't even redirect that functionality 
 to a file with another name.

So do I understand you correctly that the files we need are automatically 
copied into the workspace but since we want to use our own, old versions we 
renamed them and use these instead?

 
 /Magnus

Re: RFR: AARCH64: Top-level JDK changes

[Christian Thalinger]

 On Nov 17, 2014, at 4:11 AM, Magnus Ihse Bursie 
 magnus.ihse.bur...@oracle.com wrote:
 
 On 2014-11-14 21:44, Dean Long wrote:
 
 
 The distribution exception is there exactly since anyone should be able to 
 distribute the files with their configure script. That does not mean that 
 you are allowed to edit it, though.
 What if we require Autoconf to be installed on the host?  Does that solve 
 any problems?
 No, unfortunately not.

Why not?

 
 /Magnus

Re: RFR: AARCH64: Top-level JDK changes

[Christian Thalinger]

 On Nov 13, 2014, at 6:09 AM, Magnus Ihse Bursie 
 magnus.ihse.bur...@oracle.com wrote:
 
 On 2014-11-10 11:32, Volker Simonis wrote:
 On Mon, Nov 10, 2014 at 10:42 AM, Erik Joelsson
 erik.joels...@oracle.com wrote:
 On 2014-11-10 10:27, Volker Simonis wrote:
 On Mon, Nov 10, 2014 at 9:08 AM, Erik Joelsson erik.joels...@oracle.com
 wrote:
 Hello,
 
 I would certainly like to have these files updated, but unfortunately the
 license on these files changed from GPL2 to GPL3. This essentially means
 that the switch is non trivial from a legal perspective and the
 impression
 I've received when I last inquired about updating these files was that
 it's
 unlikely to ever happen unless a very strong case can be presented for
 why
 it's needed.
 
 So the reason we have the over engineered solution for config.guess is
 simply that it's much easier than getting legal approval for updating
 these
 files.
 OK, but in that case I don't see any reason for keeping this
 over-engineered solution at all. If there will not be any pulls from
 upstream anyway then there's no reason for keeping these file
 untouched. I'd propose then to just remove the wrappers and do all the
 chenges right in the corresponding files (of course that's not the
 topic of this change but should be done separately).
 And again, the reason we didn't change the existing file but instead wrapped
 it, was that we don't have explicit legal approval for doing derivative work
 for these 3rd party files. Maybe it's ok, maybe it's not, I will not be the
 person saying it is ok.
 
 OK, now I got it. I thought we just use the wrappers because we want
 to easily integrate the upstream versions. But instead it is only
 because we don't want to edit these files because of legal
 uncertainties.
 
 So in that case that means we're also not allowed to edit 'config.sub'
 and have to create a wrapper for it, right?
 
 Yes, you are correct. We cannot modify these files.
 
 As far as I understand, the legal reason for including these files are the 
 explicit exception:
 
 # As a special exception to the GNU General Public License, if you
 # distribute this file as part of a program that contains a
 # configuration script generated by Autoconf, you may include it under
 # the same distribution terms that you use for the rest of that program.
 
 But this is just a distribution license, not a modification license.
 
 From my IANAL point of view, this exception should be enough to disregard if 
 the file is also distributed under GPL2 or GPL3. Unfortunately, as Erik says, 
 our lawyers are apprehensive of GLP3. So while we thought that we could be 
 able to periodically sync these files with upstream (and remove our external 
 patches after a while), we have not been able to do so.

Why do we have these files in our repository in the first place?

 
 So, this fix will need to do the same dance with config.sub as for 
 guess.guess. Unfortunately. :(
 
 /Magnus

Re: RFR: AARCH64: Top-level JDK changes

[Christian Thalinger]

 On Nov 7, 2014, at 9:55 AM, Andrew Haley a...@redhat.com wrote:
 
 On 11/07/2014 05:42 PM, Christian Thalinger wrote:
 
 On Nov 7, 2014, at 9:21 AM, Andrew Haley a...@redhat.com wrote:
 
 The first patch: top-level build machinery changes.
 
 http://cr.openjdk.java.net/~aph/8064357-rev-1/ 
 http://cr.openjdk.java.net/~aph/8064357-rev-1/
 
 common/autoconf/flags.m4
 
 +aarch64)
 +  ZERO_ARCHFLAG=
 +  ;;
 
 Why is this required on aarch64 but not all the other architectures?
 
 I think it's because GCC rejects -m64”.

That’s interesting.  I thought -marchbits is some kind of common flag that 
works on all architectures.  Can someone verify this?

 
 Andrew.
 

Re: RFR: AARCH64: Top-level JDK changes

[Christian Thalinger]

 On Nov 7, 2014, at 10:10 AM, Christian Thalinger 
 christian.thalin...@oracle.com wrote:
 
 
 On Nov 7, 2014, at 9:55 AM, Andrew Haley a...@redhat.com wrote:
 
 On 11/07/2014 05:42 PM, Christian Thalinger wrote:
 
 On Nov 7, 2014, at 9:21 AM, Andrew Haley a...@redhat.com wrote:
 
 The first patch: top-level build machinery changes.
 
 http://cr.openjdk.java.net/~aph/8064357-rev-1/ 
 http://cr.openjdk.java.net/~aph/8064357-rev-1/
 
 common/autoconf/flags.m4
 
 +aarch64)
 +  ZERO_ARCHFLAG=
 +  ;;
 
 Why is this required on aarch64 but not all the other architectures?
 
 I think it's because GCC rejects -m64”.
 
 That’s interesting.  I thought -marchbits is some kind of common flag that 
 works on all architectures.  Can someone verify this?

This page doesn’t list it (while x86, SPARC, and PowerPC pages do):

https://gcc.gnu.org/onlinedocs/gcc/AArch64-Options.html 
https://gcc.gnu.org/onlinedocs/gcc/AArch64-Options.html

I guess it’s good then.

 
 
 Andrew.

Re: RFR: AARCH64: Top-level JDK changes

[Christian Thalinger]

 On Nov 7, 2014, at 10:19 AM, Andrew Haley a...@redhat.com wrote:
 
 On 11/07/2014 06:10 PM, Christian Thalinger wrote:
 
 On Nov 7, 2014, at 9:55 AM, Andrew Haley a...@redhat.com wrote:
 
 On 11/07/2014 05:42 PM, Christian Thalinger wrote:
 
 On Nov 7, 2014, at 9:21 AM, Andrew Haley a...@redhat.com wrote:
 
 The first patch: top-level build machinery changes.
 
 http://cr.openjdk.java.net/~aph/8064357-rev-1/ 
 http://cr.openjdk.java.net/~aph/8064357-rev-1/
 
 common/autoconf/flags.m4
 
 +aarch64)
 +  ZERO_ARCHFLAG=
 +  ;;
 
 Why is this required on aarch64 but not all the other architectures?
 
 I think it's because GCC rejects -m64”.
 
 That’s interesting.  I thought -marchbits is some kind of common
 flag that works on all architectures.
 
 No, all the -m stuff is target-dependent.
 
 Can someone verify this?
 
 mustang-01:~ $ gcc -m64 hello.c
 gcc: error: unrecognized command line option '-m64'
 mustang-01:~ $ gcc --version
 gcc (GCC) 4.8.2 20140120 (Red Hat 4.8.2-16)

Thanks :-)

 
 Andrew.

Re: [9] RFR(L) 8013267 : move MemberNameTable from native code to Java heap, use to intern MemberNames

[Christian Thalinger]

 On Nov 3, 2014, at 12:41 PM, David Chase david.r.ch...@oracle.com wrote:
 
 
 On 2014-11-03, at 3:09 PM, Peter Levart peter.lev...@gmail.com wrote:
 
 Hi David,
 
 I was thinking about the fact that java.lang.invoke code is leaking into 
 java.lang.Class. Perhaps, If you don't mind rewriting the code, a better 
 code structure would be, if j.l.Class changes only consisted of adding a 
 simple:
 …
 
 This way all the worries about ordering of writes into array and/or size are 
 gone. The array is still used to quickly search for an element, but VM only 
 scans the linked-list.
 
 What do you think of this?
 
 I’m not sure.  I know Coleen Ph would like to see that happen.
 
 A couple of people have vague plans to move more of the MemberName resolution 
 into core libs.
 (Years ago I worked on a VM where *all* of this occurred in Java, but some of 
 it was ahead of time.)
 
 I heard mention of “we want to put more stuff in there” but I got the 
 impression that already happened
 (there’s reflection data, for example) so I’m not sure that makes sense.
 
 There’s also a proposal from people in the runtime to just use a jmethodid, 
 take the hit of an extra indirection,
 and no need to for this worrisome jvm/java concurrency.
 
 And if we instead wrote a hash table that only grew, and never relocated 
 elements, we could
 (I think) allow non-synchronized O(1) probes of the table from the Java side, 
 synchronized
 O(1) insertions from the Java side, and because nothing moves, a smaller 
 dance with the
 VM.  I’m rather tempted to look into this — given the amount of work it would 
 take to do the
 benchmarking to see if (a) jmethodid would have acceptable performance or (b) 
 the existing
 costs are too high, I could instead just write fast code and be done.

…but you still have to do the benchmarking.  Let’s not forget that there was a 
performance regression with the first C++ implementation of this.

 
 And another way to view this is that we’re now quibbling about performance, 
 when we still
 have an existing correctness problem that this patch solves, so maybe we 
 should just get this
 done and then file an RFE.
 
 David
 

Re: MemberName$Factory.resolve() and the Eclipse debugger.

[Christian Thalinger]

 On Oct 29, 2014, at 9:39 AM, MacGregor, Duncan (GE Energy Management) 
 duncan.macgre...@ge.com wrote:
 
 When we’ve tried to debug some of our Java core in the context of running a 
 large application we’ve been seeing long pauses (sometime very long pauses of 
 over a minute) due to java.lang.invoke.MemberName$Factory.resolve() 
 apparently taking ages to complete.

Over a minute?!?  What is the JVM doing during this time?  Class loading?  
Garbage collection?

The only thing that comes to mind is that methods with breakpoints are not 
compiled but interpreted.  But even if you had a lot of breakpoints in core 
methods I don’t see how that would explain pauses of over a minute.

 Testing with an openjdk build I see the reported line number in thread dumps 
 is 962 of MemberName.java, which is where Factory.resolve() calls 
 MethodHandleNatives.resolve().
 
 Anybody else seen this, and is it even worth investigating further without 
 David’s change to intern MemberNames?
 
 Regards, Duncan.
 ___
 mlvm-dev mailing list
 mlvm-dev@openjdk.java.net
 http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

___
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

Re: MemberName$Factory.resolve() and the Eclipse debugger.

[Christian Thalinger]

 On Oct 29, 2014, at 10:06 AM, MacGregor, Duncan (GE Energy Management) 
 duncan.macgre...@ge.com wrote:
 
 On 29/10/2014 16:55, Christian Thalinger
 christian.thalin...@oracle.com wrote:
 
 On Oct 29, 2014, at 9:39 AM, MacGregor, Duncan (GE Energy Management)
 duncan.macgre...@ge.com wrote:
 
 When weąve tried to debug some of our Java core in the context of
 running a large application weąve been seeing long pauses (sometime very
 long pauses of over a minute) due to
 java.lang.invoke.MemberName$Factory.resolve() apparently taking ages to
 complete.
 
 Over a minute?!?  What is the JVM doing during this time?  Class loading?
 Garbage collection?
 
 Doesnąt seem to be doing any GC, not doing much of anything that I can
 see. Iąve applied Daivdąs MemberName interning patches and will see if
 they change the behaviour at all, and then build a JDK with debug symbols
 and see if connecting a debugger to the process sheds any light on whatąs
 going on inside the JVM.
 
 The only thing that comes to mind is that methods with breakpoints are
 not compiled but interpreted.  But even if you had a lot of breakpoints
 in core methods I donąt see how that would explain pauses of over a
 minute.
 
 It happens even without breakpoints being set. Is it possibly due to the
 avalanche of anonymous classes Lambdaforms produce?

So you’re running in Eclipse’s debugger but without breakpoints set and no 
single-stepping?

 
 Testing with an openjdk build I see the reported line number in thread
 dumps is 962 of MemberName.java, which is where Factory.resolve() calls
 MethodHandleNatives.resolve().
 
 Anybody else seen this, and is it even worth investigating further
 without Davidąs change to intern MemberNames?
 
 Regards, Duncan.
 ___
 mlvm-dev mailing list
 mlvm-dev@openjdk.java.net
 http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev
 
 ___
 mlvm-dev mailing list
 mlvm-dev@openjdk.java.net
 http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev
 
 ___
 mlvm-dev mailing list
 mlvm-dev@openjdk.java.net
 http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

___
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

Re: Loopy CallSite

[Christian Thalinger]

On Jul 16, 2014, at 11:40 AM, Remi Forax fo...@univ-mlv.fr wrote:

 
 On 07/16/2014 07:38 PM, Vladimir Ivanov wrote:
 Remi,
 
 The problem is that for every iteration you create new call site
   for(int i=0; i100_000; i++) {
 new LoopyCS().getTarget().invokeExact(1_000);
   }
 
 In LoopyCS constructor you instantiates 3 new MethodHandles:
 target = MethodHandles.filterArguments(target, 0, FOO);
 target = MethodHandles.guardWithTest(ZERO,
 target,
  MethodHandles.dropArguments(MethodHandles.constant(int.class,
  0).asType(MethodType.methodType(void.class)), 0, int.class));
 
 Since we don't cache LambdaForms yet for these combinators, 3 new 
 LambdaForms are created for each invocation. Then they are compiled to 
 bytecode and then JIT kicks in. That's why you see continuous JITing.
 
 If you reuse a call site object, it stabilizes very quickly.
 
 doh, sorry for this stupid code :(

Everyone makes mistakes once in a while.  Even you, Remi ;-)

 
 
 Best regards,
 Vladimir Ivanov
 
 Rémi
 
 
 On 7/12/14 6:05 PM, Remi Forax wrote:
 It seems that the JIT is lost with whe there is a loopy callsite and
 never stabilize (or the steady state is after the program ends).
 
 import java.lang.invoke.MethodHandle;
 import java.lang.invoke.MethodHandles;
 import java.lang.invoke.MethodType;
 import java.lang.invoke.MutableCallSite;
 
 public class Loop {
   static class LoopyCS extends MutableCallSite {
 public LoopyCS() {
   super(MethodType.methodType(void.class, int.class));
 
   MethodHandle target = dynamicInvoker();
   target = MethodHandles.filterArguments(target, 0, FOO);
   target = MethodHandles.guardWithTest(ZERO,
   target,
 MethodHandles.dropArguments(MethodHandles.constant(int.class,
 0).asType(MethodType.methodType(void.class)), 0, int.class));
   setTarget(target);
 }
   }
 
   static final MethodHandle FOO, ZERO;
   static {
 try {
   FOO = MethodHandles.lookup().findStatic(Loop.class, foo,
 MethodType.methodType(int.class, int.class));
   ZERO = MethodHandles.lookup().findStatic(Loop.class, zero,
 MethodType.methodType(boolean.class, int.class));
 } catch (NoSuchMethodException | IllegalAccessException e) {
   throw new AssertionError(e);
 }
   }
 
   private static boolean zero(int i) {
 return i != 0;
   }
 
   private static int foo(int i) {
 COUNTER++;
 return i - 1;
   }
 
   private static int COUNTER = 0;
 
   public static void main(String[] args) throws Throwable {
 for(int i=0; i100_000; i++) {
   new LoopyCS().getTarget().invokeExact(1_000);
 }
 System.out.println(COUNTER);
   }
 }
 
 cheers,
 Rémi
 
 
 ___
 mlvm-dev mailing list
 mlvm-dev@openjdk.java.net
 http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev
 ___
 mlvm-dev mailing list
 mlvm-dev@openjdk.java.net
 http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev
 
 ___
 mlvm-dev mailing list
 mlvm-dev@openjdk.java.net
 http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

___
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

Re: RFR 8047737 Move array component mirror to instance of java/lang/Class

[Christian Thalinger]

On Jun 30, 2014, at 1:06 PM, Coleen Phillimore coleen.phillim...@oracle.com 
wrote:

 
 On 6/30/14, 3:50 PM, Christian Thalinger wrote:
  private Class(ClassLoader loader) {
  // Initialize final field for classLoader.  The initialization 
 value of non-null
  // prevents future JIT optimizations from assuming this final field 
 is null.
  classLoader = loader;
 +componentType = null;
  }
 
 Are we worried about the same optimization?
 
 I don't know if I was justified in worrying about the optimization in the 
 first place.   Since getComponentType() is conditional, I wasn't worried.
 
 But it should be consistent.  Maybe I should revert the classLoader 
 constructor change, now that I fixed all the tests not to care.   What do 
 people think?
 
 +  compute_optional_offset(_component_mirror_offset,
 + klass_oop, vmSymbols::componentType_name(),
 + vmSymbols::class_signature());
 
 Is there a followup cleanup to make it non-optional?  Or, are you waiting 
 for JPRT to be able to push hotspot and jdk changes together?
 
 Yes, please look at the _full webrev.  That has the non-optional changes in 
 it and the follow on changes to remove getComponentType as an intrinsic in C2 
 (will file new RFE).  I really would like a compiler person to comment on it.

Sorry, I missed that.  Yes, the compiler changes look good.

 
 Thanks so much,
 Coleen
 
 
 On Jun 30, 2014, at 5:42 AM, Coleen Phillimore 
 coleen.phillim...@oracle.com wrote:
 
 
 On 6/30/14, 1:55 AM, David Holmes wrote:
 Hi Coleen,
 
 Your webrev links are to internal locations.
 
 Sorry, I cut/pasted the wrong links.  They are:
 
 http://cr.openjdk.java.net/~coleenp/8047737_jdk/
 http://cr.openjdk.java.net/~coleenp/8047737_hotspot/
 
 and the full version
 
 http://cr.openjdk.java.net/~coleenp/8047737_hotspot/
 
 Thank you for pointing this out David.
 
 Coleen
 
 
 David
 
 On 28/06/2014 5:24 AM, Coleen Phillimore wrote:
 Summary: Add field in java.lang.Class for componentType to simplify oop
 processing and intrinsics in JVM
 
 This is part of ongoing work to clean up oop pointers in the metadata
 and simplify the interface between the JDK j.l.C and the JVM. There's a
 performance benefit at the end of all of this if we can remove all oop
 pointers from metadata.   mirror in Klass is the only one left after
 this full change.
 
 See bug https://bugs.openjdk.java.net/browse/JDK-8047737
 
 There are a couple steps to this change because Hotspot testing is done
 with promoted JDKs.  The first step is this webrev:
 
 http://oklahoma.us.oracle.com/~cphillim/webrev/8047737_jdk/
 http://oklahoma.us.oracle.com/~cphillim/webrev/8047737_hotspot/
 
 When the JDK is promoted, the code to remove
 ArrayKlass::_component_mirror will be changed under a new bug id.
 
 http://oklahoma.us.oracle.com/~cphillim/webrev/8047737_hotspot_full
 
 Finally, a compatibility request and licensee notification will occur to
 remove the function JVM_GetComponentType.
 
 Performance testing was done that shows no difference in performance.
 The isArray() call is a compiler intrinsic which is now called instead
 of getComponentType, which was recognized as a compiler intrinsic.
 
 JDK jtreg testing, hotspot jtreg testing, hotspot NSK testing and jck8
 tests were performed on both the change requested (1st one) and the full
 change.
 
 hotspot NSK tests were run on the hotspot-only change with a promoted JDK.
 
 Please send your comments.
 
 Thanks,
 Coleen
 
 
 

Re: RFR 8047737 Move array component mirror to instance of java/lang/Class

[Christian Thalinger]

On Jun 30, 2014, at 5:50 PM, Coleen Phillimore coleen.phillim...@oracle.com 
wrote:

 
 On 6/30/14, 3:50 PM, Christian Thalinger wrote:
  private Class(ClassLoader loader) {
  // Initialize final field for classLoader.  The initialization 
 value of non-null
  // prevents future JIT optimizations from assuming this final field 
 is null.
  classLoader = loader;
 +componentType = null;
  }
 
 Are we worried about the same optimization?
 
 Hi,  I've decided to make them consistent and add another parameter to the 
 Class constructor.
 
 http://cr.openjdk.java.net/~coleenp/8047737_jdk_2/

Thanks.

 
 Thanks,
 Coleen
 
 +  compute_optional_offset(_component_mirror_offset,
 + klass_oop, vmSymbols::componentType_name(),
 + vmSymbols::class_signature());
 
 Is there a followup cleanup to make it non-optional?  Or, are you waiting 
 for JPRT to be able to push hotspot and jdk changes together?
 
 On Jun 30, 2014, at 5:42 AM, Coleen Phillimore coleen.phillim...@oracle.com 
 mailto:coleen.phillim...@oracle.com wrote:
 
 
 On 6/30/14, 1:55 AM, David Holmes wrote:
 Hi Coleen,
 
 Your webrev links are to internal locations.
 
 Sorry, I cut/pasted the wrong links.  They are:
 
 http://cr.openjdk.java.net/~coleenp/8047737_jdk/ 
 http://cr.openjdk.java.net/%7Ecoleenp/8047737_jdk/
 http://cr.openjdk.java.net/~coleenp/8047737_hotspot/
 
 and the full version
 
 http://cr.openjdk.java.net/~coleenp/8047737_hotspot/
 
 Thank you for pointing this out David.
 
 Coleen
 
 
 David
 
 On 28/06/2014 5:24 AM, Coleen Phillimore wrote:
 Summary: Add field in java.lang.Class for componentType to simplify oop
 processing and intrinsics in JVM
 
 This is part of ongoing work to clean up oop pointers in the metadata
 and simplify the interface between the JDK j.l.C and the JVM. There's a
 performance benefit at the end of all of this if we can remove all oop
 pointers from metadata.   mirror in Klass is the only one left after
 this full change.
 
 See bug https://bugs.openjdk.java.net/browse/JDK-8047737
 
 There are a couple steps to this change because Hotspot testing is done
 with promoted JDKs.  The first step is this webrev:
 
 http://oklahoma.us.oracle.com/~cphillim/webrev/8047737_jdk/
 http://oklahoma.us.oracle.com/~cphillim/webrev/8047737_hotspot/
 
 When the JDK is promoted, the code to remove
 ArrayKlass::_component_mirror will be changed under a new bug id.
 
 http://oklahoma.us.oracle.com/~cphillim/webrev/8047737_hotspot_full
 
 Finally, a compatibility request and licensee notification will occur to
 remove the function JVM_GetComponentType.
 
 Performance testing was done that shows no difference in performance.
 The isArray() call is a compiler intrinsic which is now called instead
 of getComponentType, which was recognized as a compiler intrinsic.
 
 JDK jtreg testing, hotspot jtreg testing, hotspot NSK testing and jck8
 tests were performed on both the change requested (1st one) and the full
 change.
 
 hotspot NSK tests were run on the hotspot-only change with a promoted JDK.
 
 Please send your comments.
 
 Thanks,
 Coleen
 
 
 

Re: [9] RFR(S): 8005873: JRuby test_respond_to.rb asserts with: MT-unsafe modification of inline cache

[Christian Thalinger]

On Jun 2, 2014, at 6:29 AM, Vladimir Ivanov vladimir.x.iva...@oracle.com 
wrote:

 Tobias, I'll take care of it.

Are you also taking care of the backports?

 
 Best regards,
 Vladimir Ivanov
 
 On 6/2/14 10:04 AM, Tobias Hartmann wrote:
 
 On 28.05.2014 12:48, Vladimir Ivanov wrote:
 Looks good.
 
 It should be safe to sync on MTF instance since it's not accessible
 outside (MTF and MT.form() are package-private).
 
 Best regards,
 Vladimir Ivanov
 
 Thank you, Vladimir.
 
 Could someone please push the patch?
 
 Thanks,
 Tobias
 
 
 On 5/28/14 1:49 PM, Tobias Hartmann wrote:
 Hi,
 
 thanks everyone for the feedback!
 
 @Remi: I agree with Paul. This is not a problem because if the normal
 read sees an outdated null value, a new LambdaForm is created and
 setCachedLambdaForm(...) is executed. This will guarantee that the
 non-null value is seen and used. The unnecessary creation of a new
 LamdaForm is not a problem either.
 
 @John: I added the code that you suggested to simulate CAS. Please find
 the new webrev at:
 
 http://cr.openjdk.java.net/~anoll/8005873/webrev.02/
 
 Sorry for the delay, I was on vacation.
 
 Thanks,
 Tobias
 
 On 19.05.2014 20:31, John Rose wrote:
 On May 16, 2014, at 4:56 AM, Tobias Hartmann
 tobias.hartm...@oracle.com wrote:
 
 Is it sufficient then to use synchronized (lambdaForms) { ... } in
 setCachedLambdaForm(..) and a normal read in cachedLambdaForm(..)?
 Yes, that is how I see it.  The fast path is a racy non-volatile read
 of a safely-published structure.
 
 (If safe publication via arrays were broken, java.lang.String would be
 broken.  But the JMM is carefully designed to support safe publication
 of array elements, and through array elements.)
 
 — John
 
 

hg: jdk7u/jdk7u-dev/hotspot: 2 new changesets

[christian . thalinger]

Changeset: b60f95bce6c2
Author:mikael
Date:  2014-05-29 12:40 -0700
URL:   http://hg.openjdk.java.net/jdk7u/jdk7u-dev/hotspot/rev/b60f95bce6c2

8043205: Incorrect system traps.h include path
Reviewed-by: kvn, dholmes

! src/os_cpu/linux_sparc/vm/assembler_linux_sparc.cpp

Changeset: 5a83d40edceb
Author:mikael
Date:  2014-05-29 12:38 -0700
URL:   http://hg.openjdk.java.net/jdk7u/jdk7u-dev/hotspot/rev/5a83d40edceb

8043207: Add const to Address argument for Assembler::swap
Reviewed-by: kvn, drchase

! src/cpu/sparc/vm/assembler_sparc.hpp
! src/cpu/sparc/vm/assembler_sparc.inline.hpp

hg: jdk7u/jdk7u-dev/hotspot: 2 new changesets

[christian . thalinger]

Changeset: f290ac92cb22
Author:mikael
Date:  2014-04-29 22:04 -0700
URL:   http://hg.openjdk.java.net/jdk7u/jdk7u-dev/hotspot/rev/f290ac92cb22

8022070: Compilation error in stubGenerator_sparc.cpp with some compilers
Reviewed-by: twisti, kvn

! src/cpu/sparc/vm/stubGenerator_sparc.cpp

Changeset: ca8617d66b67
Author:mikael
Date:  2014-04-29 22:05 -0700
URL:   http://hg.openjdk.java.net/jdk7u/jdk7u-dev/hotspot/rev/ca8617d66b67

8042059: Various fixes to linux/sparc
Reviewed-by: twisti, kvn

! agent/src/os/linux/libproc.h
! src/cpu/sparc/vm/frame_sparc.hpp
! src/cpu/sparc/vm/frame_sparc.inline.hpp
! src/os_cpu/linux_sparc/vm/atomic_linux_sparc.inline.hpp
! src/os_cpu/linux_sparc/vm/os_linux_sparc.cpp

Re: [9] RFR (XS): JSR 292 support for PopFrame has a fragile coupling with DirectMethodHandle

[Christian Thalinger]

Looks good.  Would be good to put a FIXME on the comment as well so we don’t 
forget to remove it.

On May 26, 2014, at 12:25 PM, Vladimir Ivanov vladimir.x.iva...@oracle.com 
wrote:

 https://bugs.openjdk.java.net/browse/JDK-8034935
 http://cr.openjdk.java.net/~vlivanov/8034935/webrev.00
 
 Citing John's explanation of motivation for this change (from the bug):
 There is a coupling from bytecodes that call MethodHandle.linkToStatic (and 
 similar linker methods) and the PopFrame feature. The linker methods accept 
 an extra appendix argument of type MemberName which is popped from the list 
 before vectoring to the desired method (it supplies the name of that method).
 
 In order to re-execute the call, the MemberName must be recovered somehow. 
 Currently, the JVM assumes that the interpreter frame's local #0 will contain 
 a DirectMethodHandle which holds the desired MemberName. The JVM should also 
 accept the MemberName itself, and eventually stop looking for the 
 DirectMethodHandle.
 
 This will simplify the handshake between JVM and JSR 292 implementation 
 bytecodes. The DMH is difficult to recover at the point of call to 
 linkToStatic, although the MemberName is guaranteed live at that point.
 
 Also, making this change (perhaps in two steps) will allow the JVM to stop 
 coupling to SystemDictionary::DirectMethodHandle_klass. Such couplings should 
 be minimized.
 
 Testing: JPRT, jtreg (java/lang/invoke), vm.mlvm.testlist
 
 Thanks!
 
 Best regards,
 Vladimir Ivanov

RFR (XS): 8032606: ClassValue.ClassValueMap.type is unused

[Christian Thalinger]

https://bugs.openjdk.java.net/browse/JDK-8032606
http://cr.openjdk.java.net/~twisti/8032606/webrev.00

8032606: ClassValue.ClassValueMap.type is unused
Reviewed-by:

Re: RFR (XS): 8032606: ClassValue.ClassValueMap.type is unused

[Christian Thalinger]

Thanks, John and Vladimir.

On May 15, 2014, at 2:46 PM, Vladimir Ivanov vladimir.x.iva...@oracle.com 
wrote:

 Looks good.
 
 Best regards,
 Vladimir Ivanov
 
 On 5/15/14 9:48 PM, Christian Thalinger wrote:
 https://bugs.openjdk.java.net/browse/JDK-8032606
 http://cr.openjdk.java.net/~twisti/8032606/webrev.00
 
 8032606: ClassValue.ClassValueMap.type is unused
 Reviewed-by:
 

Re: RFR: 8041934 com/sun/jdi/RepStep.java fails on all platforms with assert(_cur_stack_depth == count_frames()) failed: cur_stack_depth out of sync

[Christian Thalinger]

Looks good.

On May 13, 2014, at 11:58 PM, Staffan Larsen staffan.lar...@oracle.com wrote:

 Thanks Christian,
 
 I will make the change below before I push.
 
 /Staffan
 
 
 diff --git a/src/cpu/x86/vm/sharedRuntime_x86_32.cpp 
 b/src/cpu/x86/vm/sharedRuntime_x86_32.cpp
 --- a/src/cpu/x86/vm/sharedRuntime_x86_32.cpp
 +++ b/src/cpu/x86/vm/sharedRuntime_x86_32.cpp
 @@ -2248,7 +2248,7 @@
  // if we are now in interp_only_mode and in that case we do the jvmti
  // callback.
  Label skip_jvmti_method_exit;
 -__ cmpb(Address(thread, JavaThread::interp_only_mode_offset()), 0);
 +__ cmpl(Address(thread, JavaThread::interp_only_mode_offset()), 0);
  __ jcc(Assembler::zero, skip_jvmti_method_exit, true);
 
  save_native_result(masm, ret_type, stack_slots);
 diff --git a/src/cpu/x86/vm/sharedRuntime_x86_64.cpp 
 b/src/cpu/x86/vm/sharedRuntime_x86_64.cpp
 --- a/src/cpu/x86/vm/sharedRuntime_x86_64.cpp
 +++ b/src/cpu/x86/vm/sharedRuntime_x86_64.cpp
 @@ -2495,7 +2495,7 @@
  // if we are now in interp_only_mode and in that case we do the jvmti
  // callback.
  Label skip_jvmti_method_exit;
 -__ cmpb(Address(r15_thread, JavaThread::interp_only_mode_offset()), 0);
 +__ cmpl(Address(r15_thread, JavaThread::interp_only_mode_offset()), 0);
  __ jcc(Assembler::zero, skip_jvmti_method_exit, true);
 
  save_native_result(masm, ret_type, stack_slots);
 
 
 On 14 maj 2014, at 00:21, Christian Thalinger 
 christian.thalin...@oracle.com wrote:
 
 Since:
 
   int   _interp_only_mode;
 
 is an int field I would prefer to actually read the value as an int instead 
 of just a byte on x86:
 +__ cmpb(Address(r15_thread, JavaThread::interp_only_mode_offset()), 0);
 Otherwise this looks good.
 
 On May 13, 2014, at 11:30 AM, Staffan Larsen staffan.lar...@oracle.com 
 wrote:
 
 
 On 13 maj 2014, at 18:53, Daniel D. Daugherty daniel.daughe...@oracle.com 
 wrote:
 
  new webrev is here: http://cr.openjdk.java.net/~sla/8041934/webrev.02/
 
 src/share/vm/runtime/sharedRuntime.hpp
 No comments.
 
 src/share/vm/runtime/sharedRuntime.cpp
 No comments.
 
 src/cpu/sparc/vm/sharedRuntime_sparc.cpp
 No comments.
 
 src/cpu/x86/vm/sharedRuntime_x86_32.cpp
 No comments.
 
 src/cpu/x86/vm/sharedRuntime_x86_64.cpp
 No comments.
 
 On the switch from call_VM_leaf() - call_VM(), I looked through all
 the mentions of the string call_VM in the three src/cpu files. Your
 change adds the first call_VM() use in all three files and the other
 places that mention the string call_VM seem to have gone through
 a great deal of trouble not to use call_VM() directly. I have no
 specific thing I think is wrong, but I find this data worrisome…
 
 Yes, it would be great if someone from the compiler team could review this, 
 too.
 
 Thanks,
 /Staffan
 
 
 Thumbs up!
 
 Dan
 
 
 On 5/13/14 3:20 AM, Staffan Larsen wrote:
 
 On 9 maj 2014, at 20:18, serguei.spit...@oracle.com wrote:
 
 Staffan,
 
 This is important discovery, thanks!
 The fix looks good to me.
 One question below.
 
 Thanks,
 Serguei
 
 
 On 5/9/14 3:47 AM, Staffan Larsen wrote:
 On 8 maj 2014, at 19:05, Daniel D. Daugherty 
 daniel.daughe...@oracle.com wrote:
 
 webrev: http://cr.openjdk.java.net/~sla/8041934/webrev.00/
 src/share/vm/runtime/sharedRuntime.hpp
No comments.
 
 src/share/vm/runtime/sharedRuntime.cpp
line 994: JRT_LEAF(int, SharedRuntime::jvmti_method_exit(
I'm not sure that JRT_LEAF is right. I would think that
JvmtiExport::post_method_exit() would have to grab one or
more locks with all the state checks it has to make...
 
For reference, InterpreterRuntime::post_method_exit()
is a wrapper around JvmtiExport::post_method_exit()
and it is IRT_ENTRY instead of IRT_LEAF.
 Good catch!
 
 Q: Is correct to use call_VM_leaf (vs call_VM ) in the  
 sharedRuntime_arch.cpp ?
 
 +__ call_VM_leaf(
 + CAST_FROM_FN_PTR(address, SharedRuntime::jvmti_method_exit),
 + thread, rax);
 
 That is another good catch! It should probably be call_VM as you note. 
 The reason for all these leaf references is because we used the dtrace 
 probe as a template - obviously without fully understanding what we did 
 :-/
 
 I have changed to code to use call_VM instead. This also required a 
 change from jccb to jcc for the jump (which is now longer than an 8-bit 
 offset). 
 
 new webrev is here: http://cr.openjdk.java.net/~sla/8041934/webrev.02/
 
 Thanks,
 /Staffan
 
 
 
 src/cpu/sparc/vm/sharedRuntime_sparc.cpp
No comments.
 
 src/cpu/x86/vm/sharedRuntime_x86_32.cpp
No comments.
 
 src/cpu/x86/vm/sharedRuntime_x86_64.cpp
No comments.
 
 I'm guessing that PPC has the same issue, but I'm presuming
 that someone else (Vladimir?) will handle that…
 Yes, I was hoping that I could file a follow-up bug for the platforms I 
 didn’t know how to fix.
 
 Updated review: http://cr.openjdk.java.net/~sla/8041934/webrev.01/
 
 Thanks,
 /Staffan
 
 Dan
 
 
 On 5

Re: [9] RFR(S): 8005873: JRuby test_respond_to.rb asserts with: MT-unsafe modification of inline cache

[Christian Thalinger]

On May 14, 2014, at 3:47 AM, Vladimir Ivanov vladimir.x.iva...@oracle.com 
wrote:

 Tobias, I agree with your evaluation.
 
 My only concern is that @Stable doesn't work for AtomicReferenceArray, so JIT 
 doesn't see what is stored in the array.

Now that is really unfortunate.

 Maybe use a lock instead?
 
 Best regards,
 Vladimir Ivanov
 
 On 5/14/14 2:33 PM, Tobias Hartmann wrote:
 Hi Remi,
 
 sorry, I accidentally reverted that part.. Here is the correct webrev:
 
 http://cr.openjdk.java.net/~anoll/8005873/webrev.01/
 
 Thanks,
 Tobias
 
 On 14.05.2014 12:04, Remi Forax wrote:
 your patch doesn't work !
 
 the array is still allocated as an classical array in the constructor.
 
 cheers,
 Remi
 
 Envoyé avec AquaMail pour Android
 http://www.aqua-mail.com
 
 
 Le 14 mai 2014 11:04:41 Tobias Hartmann tobias.hartm...@oracle.com a
 écrit :
 
 Hi,
 
 please review the following patch for bug 8005873.
 
 *Problem*
 Bug: https://bugs.openjdk.java.net/browse/JDK-8005873
 
 The test creates multiple threads (~500) that repeatedly execute
 invokeExact on a method handle referencing a static method. The call
 to invokeExact is performed through an optimized inline cache
 (CompiledStaticCall) that points to the LambdaForm generated for the
 method handle. The IC is first set to interpreted code by
 CompiledStaticCall::set_to_interpreted(...) but soon updated to refer
 to the compiled version of the lambda form (-Xcomp).
 
 Because tiered compilation is enabled, the VM decides to compile the
 LambdaForm at a different level and sets the old version to
 not-entrant. The next call through the IC therefore triggers a
 re-resolving of the call site finally performing a Java upcall to
 java.lang.invoke.MethodHandleNatives::linkMethod(...) (see call
 sequence [1]). A *new* LambdaForm is returned and
 CompiledStaticCall::set_to_interpreted(...) is executed again to
 update the IC. The assert is hit because the callee differs.
 
 The problem is that there are multiple LambdaForms created for the
 same invokeExact instruction. Caching in the Java runtime code should
 guarantee that only one LambdaForm is created and reused.
 Instrumenting Invokers::invokeHandleForm(...) shows that almost all
 requests result in a cache miss and return a new LambdaForm.
 
 This behaviour is caused by a race condition in
 Invokers::invokeHandleForm(...). Threads may initially see a cache
 miss when invoking MethodTypeForm::cachedLambdaForm(...), then create
 a new LambdaForm and call MethodTypeForm::setCachedLambdaForm(...) to
 update the cache without checking it again. In a high concurrency
 setting, this leads to multiple LambdaForms being created for the
 same invokeExact instruction because the cache entry is overwritten
 by multiple threads.
 
 *Solution*
 Webrev: http://cr.openjdk.java.net/~anoll/8005873/webrev.00/
 
 An AtomicReferenceArray is used to cache the LambdaForms and
 .get(...) and .compareAndSet(...) are used to retrieve and update the
 cache entries. This allows only one thread to set the LambdaForm that
 is then being used by all instances of the invokeExact.
 
 *Testing*
 - Failing test (vm/mlvm/meth/stress/jni/nativeAndMH)
 - Nashorn + Octane
 - JPRT
 
 Many thanks to Christian Thalinger and Vladimir Ivanov!
 
 Best,
 Tobias
 
 [1] Call sequence of reresolving the IC target
 SharedRuntime::handle_wrong_method(...)
 - SharedRuntime::reresolve_call_site(...)
 - SharedRuntime::find_callee_method(...)
 - SharedRuntime::find_callee_info_helper(...)
 - LinkResolver::resolve_invoke(...)
 - LinkResolver::resolve_invokehandle(...)
 - LinkResolver::resolve_invokehandle(...)
 - LinkResolver::lookup_polymorphic_method(...)
 - SystemDictionary::find_method_handle_invoker(...)
 - Java upcall to java.lang.invoke.MethodHandleNatives::linkMethod(...)
 - Invokers::methodHandleInvokeLinkerMethod(...)
 - Invokers::invokeHandleForm(...)
 
 
 
 
 
 
 
 
 
 Backport?
 
 
 

Re: RFR: 8041934 com/sun/jdi/RepStep.java fails on all platforms with assert(_cur_stack_depth == count_frames()) failed: cur_stack_depth out of sync

[Christian Thalinger]

Since:

  int   _interp_only_mode;

is an int field I would prefer to actually read the value as an int instead of 
just a byte on x86:
+__ cmpb(Address(r15_thread, JavaThread::interp_only_mode_offset()), 0);
Otherwise this looks good.

On May 13, 2014, at 11:30 AM, Staffan Larsen staffan.lar...@oracle.com wrote:

 
 On 13 maj 2014, at 18:53, Daniel D. Daugherty daniel.daughe...@oracle.com 
 wrote:
 
  new webrev is here: http://cr.openjdk.java.net/~sla/8041934/webrev.02/
 
 src/share/vm/runtime/sharedRuntime.hpp
 No comments.
 
 src/share/vm/runtime/sharedRuntime.cpp
 No comments.
 
 src/cpu/sparc/vm/sharedRuntime_sparc.cpp
 No comments.
 
 src/cpu/x86/vm/sharedRuntime_x86_32.cpp
 No comments.
 
 src/cpu/x86/vm/sharedRuntime_x86_64.cpp
 No comments.
 
 On the switch from call_VM_leaf() - call_VM(), I looked through all
 the mentions of the string call_VM in the three src/cpu files. Your
 change adds the first call_VM() use in all three files and the other
 places that mention the string call_VM seem to have gone through
 a great deal of trouble not to use call_VM() directly. I have no
 specific thing I think is wrong, but I find this data worrisome…
 
 Yes, it would be great if someone from the compiler team could review this, 
 too.
 
 Thanks,
 /Staffan
 
 
 Thumbs up!
 
 Dan
 
 
 On 5/13/14 3:20 AM, Staffan Larsen wrote:
 
 On 9 maj 2014, at 20:18, serguei.spit...@oracle.com wrote:
 
 Staffan,
 
 This is important discovery, thanks!
 The fix looks good to me.
 One question below.
 
 Thanks,
 Serguei
 
 
 On 5/9/14 3:47 AM, Staffan Larsen wrote:
 On 8 maj 2014, at 19:05, Daniel D. Daugherty 
 daniel.daughe...@oracle.com wrote:
 
 webrev: http://cr.openjdk.java.net/~sla/8041934/webrev.00/
 src/share/vm/runtime/sharedRuntime.hpp
No comments.
 
 src/share/vm/runtime/sharedRuntime.cpp
line 994: JRT_LEAF(int, SharedRuntime::jvmti_method_exit(
I'm not sure that JRT_LEAF is right. I would think that
JvmtiExport::post_method_exit() would have to grab one or
more locks with all the state checks it has to make...
 
For reference, InterpreterRuntime::post_method_exit()
is a wrapper around JvmtiExport::post_method_exit()
and it is IRT_ENTRY instead of IRT_LEAF.
 Good catch!
 
 Q: Is correct to use call_VM_leaf (vs call_VM ) in the  
 sharedRuntime_arch.cpp ?
 
 +__ call_VM_leaf(
 + CAST_FROM_FN_PTR(address, SharedRuntime::jvmti_method_exit),
 + thread, rax);
 
 That is another good catch! It should probably be call_VM as you note. The 
 reason for all these leaf references is because we used the dtrace probe as 
 a template - obviously without fully understanding what we did :-/
 
 I have changed to code to use call_VM instead. This also required a change 
 from jccb to jcc for the jump (which is now longer than an 8-bit offset). 
 
 new webrev is here: http://cr.openjdk.java.net/~sla/8041934/webrev.02/
 
 Thanks,
 /Staffan
 
 
 
 src/cpu/sparc/vm/sharedRuntime_sparc.cpp
No comments.
 
 src/cpu/x86/vm/sharedRuntime_x86_32.cpp
No comments.
 
 src/cpu/x86/vm/sharedRuntime_x86_64.cpp
No comments.
 
 I'm guessing that PPC has the same issue, but I'm presuming
 that someone else (Vladimir?) will handle that…
 Yes, I was hoping that I could file a follow-up bug for the platforms I 
 didn’t know how to fix.
 
 Updated review: http://cr.openjdk.java.net/~sla/8041934/webrev.01/
 
 Thanks,
 /Staffan
 
 Dan
 
 
 On 5/8/14 12:06 AM, Staffan Larsen wrote:
 All,
 
 This is a fix for an assert in JVMTI that verifies that JVMTI’s 
 internal notion of the number of frames on the stack is correct.
 
 When running in -Xcomp mode and enable single-stepping (or 
 method_entry/exit) we will revert all frames on the stack to be run by 
 the interpreter. Only the interpreter can send single-step and 
 method_entry/exit. However, if one of the frames is a native wrapper, 
 that frame will not be reverted (presumably because we don't know how 
 to do that). This will cause us to miss a method_exit event when that 
 native frame is popped. This in turn will mess up the logic in JVMTI 
 that keeps track of the number of frames currently on the stack (see 
 JvmtiThreadState::_cur_stack_depth) and will trigger the assert.
 
 The proposed solution is to include a method_exit event in the native 
 wrapper frame if interpreted mode has been enabled. This needs updates 
 to SharedRuntime::generate_native_wrapper() for all platforms.
 
 Kudos to Rickard for helping me write the code.
 
 webrev: http://cr.openjdk.java.net/~sla/8041934/webrev.00/
 bug: https://bugs.openjdk.java.net/browse/JDK-8041934
 
 The fix has been verified by running the failing test in JPRT with 
 -Xcomp.
 
 Thanks,
 /Staffan
 
 
 
 

Re: FORK

[Christian Thalinger]

On Apr 23, 2014, at 9:44 PM, Charles Oliver Nutter head...@headius.com wrote:

 What would it take to make Hotspot forkable? Obviously we'd need to
 pause all VM threads and restarting them on the other side (or perhaps
 a prefork mode that doesn't spin up threads?) but I know there's
 challenges with signal handlers etc.
 
 I ask for a few reasons...
 
 * Dalvik has shown what you can do with a larval preforking setup.
 This is a big reason why Android apps can run in such a small amount
 of memory and start up so quickly.
 * Startup time! If we could fork an already-hot JVM, we could hit the
 ground running with *every* command, *and* still have truly separate
 processes.
 * There's a lot of development and scaling patterns that depend on
 forking, and we get constant questions about forking on JRuby.
 * Rubinius -- a Ruby VM with partially-concurrent GC, a
 signal-handling thread, JIT threads, and real parallel Ruby threads --
 supports forking. They bring the threads to a safe point, fork, and
 restart them on the other side. Color me jealous.
 
 So...given that OpenJDK is rapidly expanding into smaller-profile
 devices and new languages and development patterns, perhaps it's time
 to make it fit into the UNIX philosophy. Where do we start?

Good question.  I think you should bring this up on the hotspot-dev mailing 
list.  People from the runtime team might have some input to this (at least I 
hope; because I don’t, really).

 
 - Charlie
 ___
 mlvm-dev mailing list
 mlvm-dev@openjdk.java.net
 http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

___
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

Re: RFR (L) 8037210: Get rid of char-based descriptions 'J' of basic types

[Christian Thalinger]

On Apr 3, 2014, at 9:44 PM, John Rose john.r.r...@oracle.com wrote:

 On Apr 3, 2014, at 6:33 PM, Christian Thalinger 
 christian.thalin...@oracle.com wrote:
 
 Of course they are popular because these are the type names.  There is no 
 type L; it’s an object.  I don’t understand why we have to use different 
 names just because they are used in other namespaces.  This is not a C 
 define.
 
 They stand for JVM signatures as well as basic types.  The letters are 
 signature letters.  Can we move on from this?

Sure.  Push it.

 
 — John
 ___
 mlvm-dev mailing list
 mlvm-dev@openjdk.java.net
 http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

___
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev