Re: Running jaotc with an external Graal

[Remi Forax]

Can i say again that when using --patch-module, module-info.class should be 
merged instead of having a warning ?

Rémi

- Mail original -
> De: "Mandy Chung" 
> À: "Christian Thalinger" 
> Cc: jigsaw-dev@openjdk.java.net, "hotspot compiler" 
> , graal-...@openjdk.java.net
> Envoyé: Mercredi 15 Février 2017 23:44:19
> Objet: Re: Running jaotc with an external Graal

>> On Feb 15, 2017, at 1:56 PM, Christian Thalinger  
>> wrote:
>> 
>> 
>>> 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?
> 
> Yes it does except that module-info.class can’t be patched.  You will get a
> warning if the patched path contains module-info.class.  Are you seeing the
> otherwise?
> 
> Mandy

Re: Review Request: JDK-8173374: Update GenGraphs tool to generate dot graph with requires transitive edges

[Claes Redestad]

On 2017-02-15 22:08, Mandy Chung wrote:



On Feb 15, 2017, at 12:27 PM, Daniel Fuchs  wrote:

Hi Mandy,


Updated webrev:
  http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8173374/webrev.01/


Looks good. I haven't reviewed the build changes.
I assume they're OK if you managed to build ;-)


Thanks.

The build change is trivial adding —-add-exports for compiling and running 
GenGraphs build tool.  Also add a new target to generate spec-only module 
graphs.


+1

/Claes



Mandy

Re: Running jaotc with an external Graal

[Mandy Chung]

> On Feb 15, 2017, at 1:56 PM, Christian Thalinger  
> wrote:
> 
> 
>> 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?

Yes it does except that module-info.class can’t be patched.  You will get a 
warning if the patched path contains module-info.class.  Are you seeing the 
otherwise?

Mandy

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: Running jaotc with an external Graal

[Doug Simon]

I don’t know how one patches a module in the middle of the module graph. That 
is, we use --patch-module and --upgrade-module-path to override the 
jdk.vm.compiler module in the JDK. I don’t know what that means for modules 
such as jdk.aot that depend on jdk.vm.compiler. Maybe someone from the AOT or 
jigsaw team can help.

-Doug

> On 14 Feb 2017, at 12:26, Andrew Haley  wrote:
> 
> Is this possible?  Seems that no matter what I do, aotc always prefers to
> use the internal version of org.graalvm.compiler which is in HotSpot.
> 
> I don't quite get why this is: I can run an external Graal using JVMCI
> with no problems.  I saw "8145337: [JVMCI] JVMCI initialization with
> SecurityManager installed fails:" which might be related, but perhaps
> it's not.
> 
> So, why is it possible to use an external Graal with JVMCI, but
> apparently not with jaotc?  And is there anything I can do to make
> progress?
> 
> Thanks,
> 
> Andrew.

Re: RFR: 8175010: ImageReader is not thread-safe

[Claes Redestad]

Hi Peter,

are you suggesting that if I have class Foo { Bar b; }, then creating
and putting Foo b in a CHM before returning it to a consumer which is
then read from another thread is enough to force b to be safely
published even when the other thread does *not* get the object via a
call to the same CHM (which would go via the same volatile read and add
the necessary happens before relationship)?  I recalled having seen
examples to the effect that a non-volatile b isn't safely published in
this case.

The (very shaky) hypothesis is thus that this could be what's happening
in any of the places which load and locally cache the system ImageReader
for anyone to use, e.g., SystemModuleFinder.SystemImage or
JavaRuntimeURLConnection (which is implicitly called when there's a
security manager installed).  I might be (in fact likely am) wrong, but
we discussed this offline and came to the conclusion that there was no
harm in implementing these improvements regardless of whether they
actually resolve 8174817 or not.

I think prior to this patch a concurrent ImageReader.close() could
happen if there was a race between 3 or more threads to resolve the
same Path from ImageReaderFactory.get (especially since there might be
a longish time window there since we might block to load a library
etc), so I don't think we lose anything from plugging that hole by
using computeIfAbsent here.

I think your observations about potential issues in JRTFS is correct,
but there was nothing to suggest JRTFS code was involved in JDK-8174817
(as it's not code that's used by the BuiltinClassLoader).

Thanks!

/Claes

On 2017-02-15 21:52, Peter Levart wrote:

Hi Claes,

Reading the https://bugs.openjdk.java.net/browse/JDK-8174817 and then
the change that was just pushed, I can't seem to figure out what was the
problem with original code. I can't find evidence for claims in
https://bugs.openjdk.java.net/browse/JDK-8175010 . Is the problem
publication of ImageReader via ImageReaderFactory? That can't be it,
since ImageReaderFactory is using ConcurrentHashMap which ensures
happens before relationships.

Is there any place else where ImageReader.open(Path) is called and then
the instance unsafely published to other threads? The only place I could
find is in jdk.internal.jrtfs.SystemImage.open():

static SystemImage open() throws IOException {
if (modulesImageExists) {
// open a .jimage and build directory structure
final ImageReader image = ImageReader.open(moduleImageFile);
image.getRootDirectory();
return new SystemImage() {
@Override
Node findNode(String path) throws IOException {
return image.findNode(path);
}
@Override
byte[] getResource(Node node) throws IOException {
return image.getResource(node);
}
@Override
void close() throws IOException {
image.close();
}
};
}

...here the final 'image' local variable is captured by anonymous inner
SystemImage subclass into a synthetic final field, so this final field
ensures that ImageReader is published safely as a delegate of SystemImage.

ImageReader.open(Path) factory method delegates to
ImageReader.SharedImageReader.open(Path, ByteOrder) which creates a new
instance of ImageReader encapsulating a SharedImageReader object:

  public static ImageReader open(Path imagePath, ByteOrder
byteOrder) throws IOException {
Objects.requireNonNull(imagePath);
Objects.requireNonNull(byteOrder);

synchronized (OPEN_FILES) {
SharedImageReader reader = OPEN_FILES.get(imagePath);

if (reader == null) {
// Will fail with an IOException if wrong byteOrder.
reader =  new SharedImageReader(imagePath, byteOrder);
OPEN_FILES.put(imagePath, reader);
} else if (reader.getByteOrder() != byteOrder) {
throw new IOException("\"" + reader.getName() + "\"
is not an image file");
}

ImageReader image = new ImageReader(reader); // <<-- HERE
reader.openers.add(image);

return image;
}
}

...the ImageReader returned from this method is not safe to publish via
data race, but as far as I can see, there is no such publishing going
on. So am I right in that all this patch solves is it eliminates a
possibility of NPE when ImageReader is close()-d concurrently with being
used from some other thread. If such NPE was observed, it means that
close() is being called concurrently with access and there are still
races possible which can cause undesired effects. For example: calling
ImageReader.close() while using it concurrently may close underlying
SharedImageReader and then after closing, access it.

So is 

Re: Review Request: JDK-8173374: Update GenGraphs tool to generate dot graph with requires transitive edges

[Mandy Chung]

> On Feb 15, 2017, at 1:08 PM, Mandy Chung  wrote:
>> 
>>> Updated webrev:
>>>  http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8173374/webrev.01/

You may be interested in the module graphs generated from this change.
   http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8173374/spec-dotfiles/
   http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8173374/gengraphs/

Open jdk.graph.html if you want to view all .png files.

Mandy

Re: Review Request: JDK-8173374: Update GenGraphs tool to generate dot graph with requires transitive edges

[Mandy Chung]

> On Feb 15, 2017, at 12:27 PM, Daniel Fuchs  wrote:
> 
> Hi Mandy,
> 
> > Updated webrev:
> >   http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8173374/webrev.01/
> 
> Looks good. I haven't reviewed the build changes.
> I assume they're OK if you managed to build ;-)

Thanks.  

The build change is trivial adding —-add-exports for compiling and running 
GenGraphs build tool.  Also add a new target to generate spec-only module 
graphs.

Mandy

Re: RFR: 8175010: ImageReader is not thread-safe

[Peter Levart]

Hi Claes,

Reading the https://bugs.openjdk.java.net/browse/JDK-8174817 and then 
the change that was just pushed, I can't seem to figure out what was the 
problem with original code. I can't find evidence for claims in 
https://bugs.openjdk.java.net/browse/JDK-8175010 . Is the problem 
publication of ImageReader via ImageReaderFactory? That can't be it, 
since ImageReaderFactory is using ConcurrentHashMap which ensures 
happens before relationships.


Is there any place else where ImageReader.open(Path) is called and then 
the instance unsafely published to other threads? The only place I could 
find is in jdk.internal.jrtfs.SystemImage.open():


static SystemImage open() throws IOException {
if (modulesImageExists) {
// open a .jimage and build directory structure
final ImageReader image = ImageReader.open(moduleImageFile);
image.getRootDirectory();
return new SystemImage() {
@Override
Node findNode(String path) throws IOException {
return image.findNode(path);
}
@Override
byte[] getResource(Node node) throws IOException {
return image.getResource(node);
}
@Override
void close() throws IOException {
image.close();
}
};
}

...here the final 'image' local variable is captured by anonymous inner 
SystemImage subclass into a synthetic final field, so this final field 
ensures that ImageReader is published safely as a delegate of SystemImage.


ImageReader.open(Path) factory method delegates to 
ImageReader.SharedImageReader.open(Path, ByteOrder) which creates a new 
instance of ImageReader encapsulating a SharedImageReader object:


  public static ImageReader open(Path imagePath, ByteOrder 
byteOrder) throws IOException {

Objects.requireNonNull(imagePath);
Objects.requireNonNull(byteOrder);

synchronized (OPEN_FILES) {
SharedImageReader reader = OPEN_FILES.get(imagePath);

if (reader == null) {
// Will fail with an IOException if wrong byteOrder.
reader =  new SharedImageReader(imagePath, byteOrder);
OPEN_FILES.put(imagePath, reader);
} else if (reader.getByteOrder() != byteOrder) {
throw new IOException("\"" + reader.getName() + "\" 
is not an image file");

}

ImageReader image = new ImageReader(reader); // <<-- HERE
reader.openers.add(image);

return image;
}
}

...the ImageReader returned from this method is not safe to publish via 
data race, but as far as I can see, there is no such publishing going 
on. So am I right in that all this patch solves is it eliminates a 
possibility of NPE when ImageReader is close()-d concurrently with being 
used from some other thread. If such NPE was observed, it means that 
close() is being called concurrently with access and there are still 
races possible which can cause undesired effects. For example: calling 
ImageReader.close() while using it concurrently may close underlying 
SharedImageReader and then after closing, access it.


So is there a concurrent ImageReader.close() possible? The only place I 
see ImageReader.close() being invoked is from SystemImage.close() in the 
anonymous inner class implementation which wraps ImageReader. 
SystemImage.close() is only being invoked from JrtFileSystem.cleanup(), 
which is called from JrtFileSystem.close() and JrtFileSystem.finalize().


The following is theoretically possible:

FileSystem fs = FileSystems.newFileSystem(URI.create("jrt:/"), ...);

Path p = fs.getPath(...);
FileSystemProvider provider = fs.provider();
InputStream is = provider.newInputStream(p, ...);
// 'fs' and 'p' (which has a reference to 'fs') may be found finalizable 
and be finalized while the call to obtain content of input stream is in 
progress


For this to be prevented, the following method in JrtFileSystem:

// returns the content of the file resource specified by the path
byte[] getFileContent(JrtPath path) throws IOException {
Node node = checkNode(path);
if (node.isDirectory()) {
throw new FileSystemException(path + " is a directory");
}
//assert node.isResource() : "resource node expected here";
return image.getResource(node);
}

...would have to be changed to:

byte[] getFileContent(JrtPath path) throws IOException {
Node node = checkNode(path);
if (node.isDirectory()) {
throw new FileSystemException(path + " is a directory");
}
//assert node.isResource() : "resource node expected here";
byte[] content = image.getResource(node);
Reference.reachabilityFence(this);
return 

Re: Review Request: JDK-8173374: Update GenGraphs tool to generate dot graph with requires transitive edges

[Daniel Fuchs]

Hi Mandy,

> Updated webrev:
>   http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8173374/webrev.01/

Looks good. I haven't reviewed the build changes.
I assume they're OK if you managed to build ;-)

best regards,

-- daniel

On 15/02/17 19:32, Mandy Chung wrote:



On Feb 15, 2017, at 10:29 AM, Daniel Fuchs  wrote:

Hi Mandy,

Some early comments:

GenGraphs.java
--

 58 dir = Paths.get(args[++i]);

may produced ArrayOutOfBoundsException - should we have better
error reporting?
Or should it check && i < args.length - 1 so that it falls back
to having dir == null below?



Good catch.  Fixed to:

  i++;
  dir = i < args.length ? Paths.get(args[i]) : null;



 93 .resolve(ModuleFinder.ofSystem(),

could that be: .resolve(finder,



Fixed.



Graph.java
--

119 return builder.build().reduce();
277 this.nodes.addAll(nodes);


These were bugs, which you're taking this opportunity to fix - right?



Yes. 119 is caught by this change.  277 is caught by code inspection.



JdepsTask.java:
---

1027 // print module descriptor

Is this comment accurate?



I updated the comment:

// generate dot graph from the resolved graph from module
// resolution.  No class dependency analysis is performed.


DotFileTest.java


Missing @bug tag?



Fixed.


Updated webrev:
  http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8173374/webrev.01/

Mandy

Re: Review Request: JDK-8173374: Update GenGraphs tool to generate dot graph with requires transitive edges

[Mandy Chung]

> On Feb 15, 2017, at 10:29 AM, Daniel Fuchs  wrote:
> 
> Hi Mandy,
> 
> Some early comments:
> 
> GenGraphs.java
> --
> 
>  58 dir = Paths.get(args[++i]);
> 
> may produced ArrayOutOfBoundsException - should we have better
> error reporting?
> Or should it check && i < args.length - 1 so that it falls back
> to having dir == null below?
> 

Good catch.  Fixed to:

  i++;
  dir = i < args.length ? Paths.get(args[i]) : null;


>  93 .resolve(ModuleFinder.ofSystem(),
> 
> could that be: .resolve(finder,
> 

Fixed.

> 
> Graph.java
> --
> 
> 119 return builder.build().reduce();
> 277 this.nodes.addAll(nodes);
> 
> 
> These were bugs, which you're taking this opportunity to fix - right?
> 

Yes. 119 is caught by this change.  277 is caught by code inspection.

> 
> JdepsTask.java:
> ---
> 
> 1027 // print module descriptor
> 
> Is this comment accurate?
> 

I updated the comment:

// generate dot graph from the resolved graph from module
// resolution.  No class dependency analysis is performed.

> DotFileTest.java
> 
> 
> Missing @bug tag?
> 

Fixed.


Updated webrev:
  http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8173374/webrev.01/

Mandy

Re: RFR: 8175026: Capture build-time parameters to --generate-jli-classes

[Mandy Chung]

> On Feb 15, 2017, at 9:12 AM, Claes Redestad  wrote:
> 
> Hi,
> 
> currently the file we generate at build time as input to
> --generate-jli-classes is lost when linking custom images, which means
> user generate images may perform worse in certain ways, mostly
> generating more classes during startup.
> 
> Additionally, there's a strong assumption in --generate-jli-classes that
> the VM running jlink is going to generate compatible classes with the
> image being linked, which we can only really guarantee if the java.base
> in the linked image is of the same version as the java.base in the VM
> running jlink.  This patch tightens these checks to ensure we have
> freedom to evolve and re-evaluate the implementation in future
> releases.
> 
> JDK: http://cr.openjdk.java.net/~redestad/8175026/jdk.01/
> Top: http://cr.openjdk.java.net/~redestad/8175026/top.01/


This restriction sounds reasonable and we can enhance this in a future release. 
I think the plugin can record the configuration in its own format to be 
independent of the trace output format.

Instead of throwing ISA, you can have a test method to return a boolean to 
indicate if the default trace file should be read.

Instead of running java.base from the runtime, you can use Runtime.version() 
instead and I think comparing Version::major should be adequate. 
This change disables this default setting entirely if the image being created 
is not the same version as this plugin (defaultSpecies and defaultInvokers, 
etc).  Is it intended?

In addition, if the main argument is specified but the version does not match, 
it will ignore the given argument.  Should it be an error instead?  We are the 
one who will generate a trace file and specify it in the jlink plugin option.  
It’s okay to ignore the default trace output if no plugin option is specified 
and I think no warning should be printed in this case.  It’s just like this 
plugin is disabled.  You may want to add a suboption to turn on verbose that 
will trace what is generated and what is ignored.

Mandy

Re: Review Request: JDK-8173374: Update GenGraphs tool to generate dot graph with requires transitive edges

[Daniel Fuchs]

Hi Mandy,

Some early comments:

GenGraphs.java
--

  58 dir = Paths.get(args[++i]);

may produced ArrayOutOfBoundsException - should we have better
error reporting?
Or should it check && i < args.length - 1 so that it falls back
to having dir == null below?

  93 
.resolve(ModuleFinder.ofSystem(),


could that be: .resolve(finder,


Graph.java
--

 119 return builder.build().reduce();
 277 this.nodes.addAll(nodes);


These were bugs, which you're taking this opportunity to fix - right?


JdepsTask.java:
---

1027 // print module descriptor

Is this comment accurate?

DotFileTest.java


Missing @bug tag?

best regards,

-- daniel

On 15/02/17 00:28, Mandy Chung wrote:

http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8173374/webrev.00/

This is the first step to enable generating dot graph to be
included in module summary javadoc, if desired.  jdeps already
supports generating the dot graph for modules.  This patch
converts GenGraphs build tool to use jdeps implementation
as well as fixes jdeps to work with -apionly to generate
a dot graph containing `requires transitive` edges only.

Mandy
[1] https://bugs.openjdk.java.net/browse/JDK-8173303

RFR: 8175026: Capture build-time parameters to --generate-jli-classes

[Claes Redestad]

Hi,

currently the file we generate at build time as input to
--generate-jli-classes is lost when linking custom images, which means
user generate images may perform worse in certain ways, mostly
generating more classes during startup.

Additionally, there's a strong assumption in --generate-jli-classes that
the VM running jlink is going to generate compatible classes with the
image being linked, which we can only really guarantee if the java.base
in the linked image is of the same version as the java.base in the VM
running jlink.  This patch tightens these checks to ensure we have
freedom to evolve and re-evaluate the implementation in future
releases.

JDK: http://cr.openjdk.java.net/~redestad/8175026/jdk.01/
Top: http://cr.openjdk.java.net/~redestad/8175026/top.01/
Bug: https://bugs.openjdk.java.net/browse/JDK-8175026

Thanks!

/Claes

Re: Extending java.base module

[Volker Simonis]

On Wed, Feb 15, 2017 at 5:16 PM, Alan Bateman  wrote:
> On 15/02/2017 16:01, Daniel Fuchs wrote:
>
>>
>> In that specific case it's not java.base that depends
>> on java.security.jgss, but the application itself.
>>
>> So I would expect the application code to either require
>> java.security.jgss, or some higher level module for that
>> itself requires java.security.jgss, or jlink to be run with
>> command line options that explicitly add java.security.jgss
>> to the image.
>
> java.security.jgss exports an API so it will be resolved by default when the
> initial class is loaded from the class path. In addition, it provides a
> SecurityProvider implementation and so will be resolved because java.base
> `uses java.security.Provider`. For the jlink case then you are right, it
> needs someone to know that the application might need to do SPNEGO
> authentication.
>
> In any case, it's an example of how not to do things, and hopefully it will
> be cleaned up at some point.
>

Daniel, Alan, thanks for the clarification. I didn't wanted to blame
anybody - just looking for good arguments to prevent such code in our
version of the JDK :)

> -Alan
>
>

Re: Extending java.base module

[Alan Bateman]

On 15/02/2017 16:01, Daniel Fuchs wrote:



In that specific case it's not java.base that depends
on java.security.jgss, but the application itself.

So I would expect the application code to either require
java.security.jgss, or some higher level module for that
itself requires java.security.jgss, or jlink to be run with
command line options that explicitly add java.security.jgss
to the image.
java.security.jgss exports an API so it will be resolved by default when 
the initial class is loaded from the class path. In addition, it 
provides a SecurityProvider implementation and so will be resolved 
because java.base `uses java.security.Provider`. For the jlink case then 
you are right, it needs someone to know that the application might need 
to do SPNEGO authentication.


In any case, it's an example of how not to do things, and hopefully it 
will be cleaned up at some point.


-Alan

Re: RFR: 8175010: ImageReader is not thread-safe

[Mandy Chung]

> On Feb 15, 2017, at 5:22 AM, Claes Redestad  wrote:
> 
> Hi,
> 
> a few intermittent but rare test failures[1] that has appeared
> since the latest jake integration, and since one of the changes
> in there was to make initialization of the system ImageReader
> lazy there appears to be cases where ImageReaders are not
> safely published:
> 
> - Ensure ImageReader::open is called only once per Path in
> ImageReaderFactory by using CHM.computeIfAbsent
> - Ensure ImageReader.reader is safely published to a
> final field and signal close using a volatile boolean instead
> 
> webrev: http://cr.openjdk.java.net/~redestad/8175010/webrev.02/

Looks good.

Mandy

Re: Extending java.base module

[Daniel Fuchs]

Hi Volker,

On 15/02/17 15:52, Volker Simonis wrote:

Hi Max,

I'm not an jigsaw either, but wouldn't your solution break a tool like jlink?

In other words, if an application uses your code and the developer
uses jlink to create a run-time image, wouldn't that image fail to
execute his application because jlink fails to see that java.base
depends on java.security.jgss in that special case?


In that specific case it's not java.base that depends
on java.security.jgss, but the application itself.

So I would expect the application code to either require
java.security.jgss, or some higher level module for that
itself requires java.security.jgss, or jlink to be run with
command line options that explicitly add java.security.jgss
to the image.

best regards,

-- daniel



Thanks,
Volker

Re: Extending java.base module

[Volker Simonis]

Hi Max,

I'm not an jigsaw either, but wouldn't your solution break a tool like jlink?

In other words, if an application uses your code and the developer
uses jlink to create a run-time image, wouldn't that image fail to
execute his application because jlink fails to see that java.base
depends on java.security.jgss in that special case?

Thanks,
Volker


On Wed, Feb 15, 2017 at 9:51 AM, Weijun Wang  wrote:
> Disclaimer: I am not a jigsaw expert.
>
> The provides/uses mechanism is certainly more formal, but you can also do
> http://hg.openjdk.java.net/jdk9/dev/jdk/rev/d282c1a8d20b.
>
> --Max
>
>
> On 02/15/2017 04:36 PM, Langer, Christoph wrote:
>>
>> Hi Jigsaw experts,
>>
>> as you might or might not know, we have an own JDK implementation with
>> some extension code that is quite interwoven with the jdk.
>>
>> Now I'm looking into how this coding can be spread into a good module
>> structure for jdk9. And I'm not a crack yet on using the module system
>> though I've read quite a bit into the spec documentation available so far;-)
>>
>> The first point for me is that we have to place some of our coding in the
>> java.base module as we used to add private fields and methods to basic
>> classes such as java.lang.Thread or java.lang.Exception. However, I don't
>> want to have so much of our stuff in java.base and rather think that it
>> should live in its own module. So the question here is if it is possible to
>> call code of other modules from java.base, e.g. via the Service Provider
>> interface? I see that I can define a service in java.base and specify some
>> "uses" statement in module-info. But will my implementation of such a
>> service from other modules be available to java.base?
>>
>> Also I'm contemplating about this requirement: I have a class which I
>> would need somewhere in java.base but I'd also like to export it in the
>> public API of my own extension module. So, if I create the class in
>> java.base, I'm not allowed to export this class publicly, unqualified,
>> right? But when I have it living in my extension module, then java.base
>> would not see it. What can I do? Probably create some inherited class in my
>> extension module that extends from the java.base impl and export this??
>>
>> I'm hoping that those are easy questions for you and you can give me some
>> helpful answers.
>>
>> Thanks a lot in advance!!
>>
>> Best regards
>> Christoph
>>
>

Re: RFR: 8175010: ImageReader is not thread-safe

[Claes Redestad]

Jim, Chris, Alan, thanks for reviewing!

On 02/15/2017 02:56 PM, Alan Bateman wrote:

On 15/02/2017 13:22, Claes Redestad wrote:


Hi,

a few intermittent but rare test failures[1] that has appeared
since the latest jake integration, and since one of the changes
in there was to make initialization of the system ImageReader
lazy there appears to be cases where ImageReaders are not
safely published:

- Ensure ImageReader::open is called only once per Path in
ImageReaderFactory by using CHM.computeIfAbsent
- Ensure ImageReader.reader is safely published to a
final field and signal close using a volatile boolean instead

webrev: http://cr.openjdk.java.net/~redestad/8175010/webrev.02/
bug: https://bugs.openjdk.java.net/browse/JDK-8175010

ImageReaderFactory looks good.

The changes to ImageReader are okay too, always a bit odd that this 
code throw NPE when the reader was closed. There is still an issue 
with async close of course in that someone could close at the same 
time as an access. However that is a high-level issue for jrtfs, at 
run-time then the image file is opened once and is never closed.


Yes, this patch doesn't attempt to improve on how races are handled when 
closing, but should help to avoid potential publication issues when opening.


It seems it could be worthwhile to have a special implementation class 
for the system module, since we could make it perfectly shareable and 
non-closeable, thus avoid delegation and closed-checking overheads, but 
that'd be a larger endeavor, perhaps suitable as an RFE for 10.


/Claes



-Alan

Re: RFR: 8175010: ImageReader is not thread-safe

[Alan Bateman]

On 15/02/2017 13:22, Claes Redestad wrote:


Hi,

a few intermittent but rare test failures[1] that has appeared
since the latest jake integration, and since one of the changes
in there was to make initialization of the system ImageReader
lazy there appears to be cases where ImageReaders are not
safely published:

- Ensure ImageReader::open is called only once per Path in
ImageReaderFactory by using CHM.computeIfAbsent
- Ensure ImageReader.reader is safely published to a
final field and signal close using a volatile boolean instead

webrev: http://cr.openjdk.java.net/~redestad/8175010/webrev.02/
bug: https://bugs.openjdk.java.net/browse/JDK-8175010

ImageReaderFactory looks good.

The changes to ImageReader are okay too, always a bit odd that this code 
throw NPE when the reader was closed. There is still an issue with async 
close of course in that someone could close at the same time as an 
access. However that is a high-level issue for jrtfs, at run-time then 
the image file is opened once and is never closed.


-Alan

Re: Java SE JSR 250 annotations module renamed to java.xml.ws.annotation?

[David M. Lloyd]

On 02/14/2017 05:52 PM, mark.reinh...@oracle.com wrote:

2017/2/13 9:17:47 -0800, Guillaume Smet :

On Mon, Feb 13, 2017 at 6:10 PM, alan.bate...@oracle.com wrote:

On 13/02/2017 16:58, Guillaume Smet wrote:

On Mon, Feb 13, 2017 at 5:12 PM, alan.bate...@oracle.com wrote:


I agree that @Generated is awkward but I haven't suggested removing it.


What do you suggest then? As far as I understood you, you were suggesting
removing the module in Java 10 so the @Generated annotation would also be
gone? Or did I misunderstand?


That is the proposal. If it goes ahead then it means that tools that rely
on these annotations in the JDK would need to deploy the standalone version
on the class path or as a module on the module path.


Yeah, so basically, it would end up with
http://hg.openjdk.java.net/code-tools/jmh/rev/d74b2861222c .

I don't think it's the best possible outcome for this useful annotation.


I agree.

The `@Generated` annotation falls outside the original charter of the
`java.lang.annotation` package, which was meant for annotations that
directly support the language's annotation facility, but we already
added `@Native` in SE 8, so let's add `@Generated` in SE 9 as David
suggests and encourage people to use it when running on this and later
releases.

The fact that `@Generated` is so widely used is new information to some
of us, so thanks for bringing it up.


I'm glad to hear it... thanks!

--
- DML

Re: RFR: 8175010: ImageReader is not thread-safe

[Chris Hegarty]

> On 15 Feb 2017, at 13:22, Claes Redestad  wrote:
> 
> Hi,
> 
> a few intermittent but rare test failures[1] that has appeared
> since the latest jake integration, and since one of the changes
> in there was to make initialization of the system ImageReader
> lazy there appears to be cases where ImageReaders are not
> safely published:
> 
> - Ensure ImageReader::open is called only once per Path in
> ImageReaderFactory by using CHM.computeIfAbsent
> - Ensure ImageReader.reader is safely published to a
> final field and signal close using a volatile boolean instead
> 
> webrev: http://cr.openjdk.java.net/~redestad/8175010/webrev.02/

Looks good Claes.

-Chris.

> bug: https://bugs.openjdk.java.net/browse/JDK-8175010
> 
> Testing shows no issues (which admittedly doesn't mean we're
> actually solving the root cause for JDK-8174817), and performance
> numbers from adding a volatile read indicate that any overhead
> is lost in the noise on ImageReader-heavy workloads.
> 
> Thanks!
> 
> /Claes
> 
> [1] https://bugs.openjdk.java.net/browse/JDK-8174817

Re: RFR: 8175010: ImageReader is not thread-safe

[Jim Laskey (Oracle)]

+1

> On Feb 15, 2017, at 9:22 AM, Claes Redestad  wrote:
> 
> Hi,
> 
> a few intermittent but rare test failures[1] that has appeared
> since the latest jake integration, and since one of the changes
> in there was to make initialization of the system ImageReader
> lazy there appears to be cases where ImageReaders are not
> safely published:
> 
> - Ensure ImageReader::open is called only once per Path in
> ImageReaderFactory by using CHM.computeIfAbsent
> - Ensure ImageReader.reader is safely published to a
> final field and signal close using a volatile boolean instead
> 
> webrev: http://cr.openjdk.java.net/~redestad/8175010/webrev.02/
> bug: https://bugs.openjdk.java.net/browse/JDK-8175010
> 
> Testing shows no issues (which admittedly doesn't mean we're
> actually solving the root cause for JDK-8174817), and performance
> numbers from adding a volatile read indicate that any overhead
> is lost in the noise on ImageReader-heavy workloads.
> 
> Thanks!
> 
> /Claes
> 
> [1] https://bugs.openjdk.java.net/browse/JDK-8174817

Re: Extending java.base module

[Michael Rasmussen]

>
> E.g. if I need to register/reach my service already at the early stages of
> JVM initialization, e.g. when a class java.lang.Thread gets initialized,
> can I assume a service from my extension module would be available?
>
No. At that time only java.base classes can be loaded.

If you look at the comments in the initPhase# methods in System, it gives
some good info about when things are initialized.

http://hg.openjdk.java.net/jdk9/jdk9/jdk/file/tip/src/java.base/share/classes/java/lang/System.java#l1850

/Michael

Re: Extending java.base module

[Alan Bateman]

On 15/02/2017 12:10, David Holmes wrote:

On 15/02/2017 8:03 PM, Langer, Christoph wrote:

Hi Chris, Max,

thanks for your quick answers. So the service approach seems to fit 
quite well.


But can I assume that my service implementation will be available 
already at "bootstrap time" of the JDK? E.g. if I need to 
register/reach my service already at the early stages of JVM 
initialization, e.g. when a class java.lang.Thread gets initialized, 
can I assume a service from my extension module would be available?


I'm pretty sure the answer to that will be No! Thread is one of the 
earlier classes to be initialized, the module system is initialized 
much later.
That's right as only classes in java.base can be loaded during startup. 
I don't know what services that Christoph is thinking of but hopefully 
they can be deferred until the VM is initialized.


-Alan

Re: Extending java.base module

[David Holmes]

On 15/02/2017 8:03 PM, Langer, Christoph wrote:

Hi Chris, Max,

thanks for your quick answers. So the service approach seems to fit quite well.

But can I assume that my service implementation will be available already at 
"bootstrap time" of the JDK? E.g. if I need to register/reach my service 
already at the early stages of JVM initialization, e.g. when a class java.lang.Thread 
gets initialized, can I assume a service from my extension module would be available?


I'm pretty sure the answer to that will be No! Thread is one of the 
earlier classes to be initialized, the module system is initialized much 
later.


David
-


Thanks,
Christoph



-Original Message-
From: Chris Hegarty [mailto:chris.hega...@oracle.com]
Sent: Mittwoch, 15. Februar 2017 10:04
To: Weijun Wang 
Cc: Langer, Christoph ; jigsaw-
d...@openjdk.java.net
Subject: Re: Extending java.base module



On 15 Feb 2017, at 08:51, Weijun Wang  wrote:

Disclaimer: I am not a jigsaw expert.

The provides/uses mechanism is certainly more formal, but you can also do

http://hg.openjdk.java.net/jdk9/dev/jdk/rev/d282c1a8d20b.

This is, at best, a hack. The use of Services is a better approach, where
possible.

-Chris.

Re: Java SE JSR 250 annotations module renamed to java.xml.ws.annotation?

[Remi Forax]

I agree.

Remi


On February 15, 2017 12:52:53 AM GMT+01:00, mark.reinh...@oracle.com wrote:
>2017/2/13 9:17:47 -0800, Guillaume Smet :
>> On Mon, Feb 13, 2017 at 6:10 PM, alan.bate...@oracle.com wrote:
>>> On 13/02/2017 16:58, Guillaume Smet wrote:
 On Mon, Feb 13, 2017 at 5:12 PM, alan.bate...@oracle.com wrote:
> 
> I agree that @Generated is awkward but I haven't suggested
>removing it.
 
 What do you suggest then? As far as I understood you, you were
>suggesting
 removing the module in Java 10 so the @Generated annotation would
>also be
 gone? Or did I misunderstand?
>>> 
>>> That is the proposal. If it goes ahead then it means that tools that
>rely
>>> on these annotations in the JDK would need to deploy the standalone
>version
>>> on the class path or as a module on the module path.
>> 
>> Yeah, so basically, it would end up with
>> http://hg.openjdk.java.net/code-tools/jmh/rev/d74b2861222c .
>> 
>> I don't think it's the best possible outcome for this useful
>annotation.
>
>I agree.
>
>The `@Generated` annotation falls outside the original charter of the
>`java.lang.annotation` package, which was meant for annotations that
>directly support the language's annotation facility, but we already
>added `@Native` in SE 8, so let's add `@Generated` in SE 9 as David
>suggests and encourage people to use it when running on this and later
>releases.
>
>The fact that `@Generated` is so widely used is new information to some
>of us, so thanks for bringing it up.
>
>- Mark

-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

Re: Extending java.base module

[Alan Bateman]

On 15/02/2017 08:36, Langer, Christoph wrote:


Hi Jigsaw experts,

as you might or might not know, we have an own JDK implementation with some 
extension code that is quite interwoven with the jdk.

Now I'm looking into how this coding can be spread into a good module structure 
for jdk9. And I'm not a crack yet on using the module system though I've read 
quite a bit into the spec documentation available so far;-)

The first point for me is that we have to place some of our coding in the java.base 
module as we used to add private fields and methods to basic classes such as 
java.lang.Thread or java.lang.Exception. However, I don't want to have so much of our 
stuff in java.base and rather think that it should live in its own module. So the 
question here is if it is possible to call code of other modules from java.base, e.g. via 
the Service Provider interface? I see that I can define a service in java.base and 
specify some "uses" statement in module-info. But will my implementation of 
such a service from other modules be available to java.base?

Also I'm contemplating about this requirement: I have a class which I would 
need somewhere in java.base but I'd also like to export it in the public API of 
my own extension module. So, if I create the class in java.base, I'm not 
allowed to export this class publicly, unqualified, right? But when I have it 
living in my extension module, then java.base would not see it. What can I do? 
Probably create some inherited class in my extension module that extends from 
the java.base impl and export this??

I'm hoping that those are easy questions for you and you can give me some 
helpful answers.

Yes, services is the way to do this. The jdk.net module is one example, 
there are several others. The other thing to be aware of is the 
module-info.java.extra files to augment the module declarations during 
the build, I suspect you'll end up using that.


-Alan

RE: Extending java.base module

[Langer, Christoph]

Hi Chris, Max,

thanks for your quick answers. So the service approach seems to fit quite well.

But can I assume that my service implementation will be available already at 
"bootstrap time" of the JDK? E.g. if I need to register/reach my service 
already at the early stages of JVM initialization, e.g. when a class 
java.lang.Thread gets initialized, can I assume a service from my extension 
module would be available?

Thanks,
Christoph


> -Original Message-
> From: Chris Hegarty [mailto:chris.hega...@oracle.com]
> Sent: Mittwoch, 15. Februar 2017 10:04
> To: Weijun Wang 
> Cc: Langer, Christoph ; jigsaw-
> d...@openjdk.java.net
> Subject: Re: Extending java.base module
> 
> 
> > On 15 Feb 2017, at 08:51, Weijun Wang  wrote:
> >
> > Disclaimer: I am not a jigsaw expert.
> >
> > The provides/uses mechanism is certainly more formal, but you can also do
> http://hg.openjdk.java.net/jdk9/dev/jdk/rev/d282c1a8d20b.
> 
> This is, at best, a hack. The use of Services is a better approach, where
> possible.
> 
> -Chris.

Re: Extending java.base module

[Chris Hegarty]

> On 15 Feb 2017, at 08:51, Weijun Wang  wrote:
> 
> Disclaimer: I am not a jigsaw expert.
> 
> The provides/uses mechanism is certainly more formal, but you can also do 
> http://hg.openjdk.java.net/jdk9/dev/jdk/rev/d282c1a8d20b.

This is, at best, a hack. The use of Services is a better approach, where 
possible.

-Chris.

Re: Extending java.base module

[Weijun Wang]

Disclaimer: I am not a jigsaw expert.

The provides/uses mechanism is certainly more formal, but you can also 
do http://hg.openjdk.java.net/jdk9/dev/jdk/rev/d282c1a8d20b.


--Max

On 02/15/2017 04:36 PM, Langer, Christoph wrote:

Hi Jigsaw experts,

as you might or might not know, we have an own JDK implementation with some 
extension code that is quite interwoven with the jdk.

Now I'm looking into how this coding can be spread into a good module structure 
for jdk9. And I'm not a crack yet on using the module system though I've read 
quite a bit into the spec documentation available so far;-)

The first point for me is that we have to place some of our coding in the java.base 
module as we used to add private fields and methods to basic classes such as 
java.lang.Thread or java.lang.Exception. However, I don't want to have so much of our 
stuff in java.base and rather think that it should live in its own module. So the 
question here is if it is possible to call code of other modules from java.base, e.g. via 
the Service Provider interface? I see that I can define a service in java.base and 
specify some "uses" statement in module-info. But will my implementation of 
such a service from other modules be available to java.base?

Also I'm contemplating about this requirement: I have a class which I would 
need somewhere in java.base but I'd also like to export it in the public API of 
my own extension module. So, if I create the class in java.base, I'm not 
allowed to export this class publicly, unqualified, right? But when I have it 
living in my extension module, then java.base would not see it. What can I do? 
Probably create some inherited class in my extension module that extends from 
the java.base impl and export this??

I'm hoping that those are easy questions for you and you can give me some 
helpful answers.

Thanks a lot in advance!!

Best regards
Christoph

Extending java.base module

[Langer, Christoph]

Hi Jigsaw experts,

as you might or might not know, we have an own JDK implementation with some 
extension code that is quite interwoven with the jdk.

Now I'm looking into how this coding can be spread into a good module structure 
for jdk9. And I'm not a crack yet on using the module system though I've read 
quite a bit into the spec documentation available so far;-)

The first point for me is that we have to place some of our coding in the 
java.base module as we used to add private fields and methods to basic classes 
such as java.lang.Thread or java.lang.Exception. However, I don't want to have 
so much of our stuff in java.base and rather think that it should live in its 
own module. So the question here is if it is possible to call code of other 
modules from java.base, e.g. via the Service Provider interface? I see that I 
can define a service in java.base and specify some "uses" statement in 
module-info. But will my implementation of such a service from other modules be 
available to java.base?

Also I'm contemplating about this requirement: I have a class which I would 
need somewhere in java.base but I'd also like to export it in the public API of 
my own extension module. So, if I create the class in java.base, I'm not 
allowed to export this class publicly, unqualified, right? But when I have it 
living in my extension module, then java.base would not see it. What can I do? 
Probably create some inherited class in my extension module that extends from 
the java.base impl and export this??

I'm hoping that those are easy questions for you and you can give me some 
helpful answers.

Thanks a lot in advance!!

Best regards
Christoph