Re: Module exports for java.base: NIO stuff

2016-12-11 Thread Alan Bateman

On 11/12/2016 22:38, David M. Lloyd wrote:



I'm not implementing SelectorProvider, I just want access to all the 
possible implementations that are available in the current JDK so we 
can select among them on a case-by-case basis.



Are you attempting
to wrap the default implementation - I ask because there isn't anything
in the service type (SelectorProvider in this example) that is useful
for doing selection.


Except for the name of the class, which is what we select by today.
At least in JDK 9 then the only platform in OpenJDK (or Oracle JDK 
builds) where there is more than one implementation is Solaris (it has a 
/dev/poll and Solaris I/O ports based implementations). Maybe XNIO wants 
to use the legacy poll based Selector on Mondays? It will be out of luck 
in JDK 9 as that implementation is excluded by the build (it wasn't 
deleted because some people wanted to keep it around for bootstrapping 
new ports). So I will guess that SelectorProvider::provider gets you 
most of the way. Maybe this request is rooted in the proposal to add a 
poll method to SocketChannel that didn't come to a timely conclusion?


In any case, what you are asking for would require a spec update. 
Specifically SelectorProvider::provider would need to specify that it 
ignores "built-in" implementations that are located via ServiceLoader. 
It would need an implementation changes too. Nothing too difficult of 
course but it just not something that has ever come up before (at least 
to my knowledge).


-Alan


Re: Use cases for java.lang.Module.addExports

2016-12-11 Thread Alan Bateman

On 12/12/2016 06:34, David Holmes wrote:

Does addExports only affect reflective access to the module, or does 
it change all access? I'm trying to determine what the set of actions 
X will be such that:


try {
  ;
}
catch () {
  execute( () -> mod.addExports(...)); // address the problem
  ;  // now it succeeds
} 
It updates the module to export the package so that public types in that 
package are accessible to bytecode, code reflection, also method handles.


As to what  is then it might be an access from bytecode (maybe 
generated code or just a mismatch between the compile time and run time 
environments that leads to IllegalAccessError).


More likely then  is using core reflection and an access check with 
Constructor::newInstance, Method::invoke or Field::set fails with 
IllegalAccessException. The retry in your example will succeed (assuming 
the package is now exported to at least the caller and access is 
otherwise allowed by the class/member modifiers).  could be something 
using method handles too, say where a Lookup::find fails 
IllegalAccessException.


-Alan




Use cases for java.lang.Module.addExports

2016-12-11 Thread David Holmes
Does addExports only affect reflective access to the module, or does it 
change all access? I'm trying to determine what the set of actions X 
will be such that:


try {
  ;
}
catch () {
  execute( () -> mod.addExports(...)); // address the problem
  ;  // now it succeeds
}

Thanks,
David


Re: RFR 8168925: MODULES property should be topologically ordered and space-separated list

2016-12-11 Thread Sundararajan Athijegannathan

Hi Mandy,

I'll refactor (and revisit) TOP/OTHER types in my next jlink change. 
Thanks for the review.


Thanks,
-Sundar


On 12/12/16, 11:04 AM, Mandy Chung wrote:

On Dec 11, 2016, at 9:30 PM, Sundararajan 
Athijegannathan  wrote:

Hi,

Please review the updated webrev: 
http://cr.openjdk.java.net/~sundar/8168925/webrev.02/

This looks better.  Thanks for making the change.

One thing to mention is that the storeRelease(ResourcePool pool) method to 
write the release file because the accept(ResourcePoolEntry file) method 
excludes “/java.base/release” entry:

  384 case TOP:
  385 break;

This release file entry could have been written in the same way as other 
entries now and no need for this storeRelease method.  I’m okay if you want to 
leave this as is and revisit the TOP and OTHER entry type in the future.

Mandy


Re: RFR 8168925: MODULES property should be topologically ordered and space-separated list

2016-12-11 Thread Mandy Chung

> On Dec 11, 2016, at 9:30 PM, Sundararajan Athijegannathan 
>  wrote:
> 
> Hi,
> 
> Please review the updated webrev: 
> http://cr.openjdk.java.net/~sundar/8168925/webrev.02/

This looks better.  Thanks for making the change.

One thing to mention is that the storeRelease(ResourcePool pool) method to 
write the release file because the accept(ResourcePoolEntry file) method 
excludes “/java.base/release” entry:

 384 case TOP:
 385 break;

This release file entry could have been written in the same way as other 
entries now and no need for this storeRelease method.  I’m okay if you want to 
leave this as is and revisit the TOP and OTHER entry type in the future.

Mandy

Re: RFR 8168925: MODULES property should be topologically ordered and space-separated list

2016-12-11 Thread Sundararajan Athijegannathan

Hi,

Please review the updated webrev: 
http://cr.openjdk.java.net/~sundar/8168925/webrev.02/


Changes from the last review:

* Moved all release file property filling code from 
DefaultImageBuilder.java to ReleaseInfoPlugin.java
* DefaultImageBuilder populates this.targetOsName directly from 
java.base module

* Added a comment for CATEGORIES_ORDER in ImagePluginConfiguration.java
* In test ModuleNamesOrderTest.java, changed moduleName to be output dir 
name ("image..." name)


Thanks,
-Sundar

On 10/12/16, 11:27 AM, Mandy Chung wrote:

On Dec 9, 2016, at 7:32 PM, Sundararajan 
Athijegannathan  wrote:

Hi,

Thanks for your review. Comments below..

On 10/12/16, 2:18 AM, Mandy Chung wrote:

On Dec 9, 2016, at 12:49 AM, Sundararajan 
Athijegannathan   wrote:

Please review http://cr.openjdk.java.net/~sundar/8168925/webrev.01/index.html 
for https://bugs.openjdk.java.net/browse/JDK-8168925


Is the order of Plugin.Category enums significant?  You moved COMPRESSOR down - 
is it necessary?

Yes. Without that ReleaseInfoPlugin will receive compressed resources and 
compressed module-info.class won't be parsed okay by ModuleDescriptor.read 
(called by ModuleSorter). Note that auto-decompression is done only by 
LastResourcePool - which is created after all plugins operate. [I kept getting 
test failures and debugged to find this is the cause of failures!]


OK.  If the order of the enums determines the plugin ordering, it’d be good 
adding a comment.


Can you look at DefaultImageBuilder::releaseProperties which I think this should be 
moved to ReleaseInfoPlugin?  The content of `release` should be written when the new 
entry "/java.base/release” is added to the resource pool.  DefaultImageBuilder 
does not need to add any more properties to this `release` file as all properties in 
`release` are known once the graph is resolved.

Okay, I'll check if I can move all stuff there. From initial communication 
[private email], I thought the suggestion was only about MODULES.

This has beeen my suggestion - move the whole thing out from 
DefaultImageBuilder.  I don’t see the reason why it has to be done in two 
separate places.  That’ll be a good clean up.



src/jdk.jlink/share/classes/jdk/tools/jlink/internal/ResourcePoolManager.java
- is this change needed?

Yes. Helped me debugging to find out which module's module-info.class was not 
parsed fine. Exception translation puts the name of the module in the message - 
which will be missing in the exception thrown by 
ModuleDescriptor.read(ByteBuffer).

OK.


test/tools/jlink/CustomPluginTest.java
It’s one option to disable the releaseinfo plugin.  An alternative
way to fix this test is [2].

I could. But that test checks for the module dependency checks that happen 
after all plugins are exercised. That should happen regardless of release-info 
plugin is enabled. i.e., even when release-info plugin is disabled, that module 
missing error should be thrown and the test checks for that.


OK.  If I happen to push JDK-8169925 before this, you can revert my change to 
this test.

Thanks
Mandy


Re: Proposal: #VersionedDependences

2016-12-11 Thread Stephen Colebourne
On 9 December 2016 at 21:46,   wrote:
> When compiling a module that depends on some other modules, record the
> version strings of those modules, if available, in the resulting module
> descriptor.

Overall this seems like a good idea, for diagnostic reasons.

> Now that compile-time versions can be recorded in module descriptors
> there is even less need to tolerate version information in module names,
> a bad practice that we'd like to discourage at the outset.  We therefore
> further propose to:
>
>   - Revise the accepted proposal for #VersionsInModuleNames [3] to state
> that a module name appearing anywhere in a source-form module
> declaration must both start and end with "Java letters" [4].

I continue to think that this is unwise. There are legitimate reasons
for projects to want to have a number at the end of the project name -
fabric8 and lang3 being mentioned already.

Stephen


Re: Module exports for java.base: NIO stuff

2016-12-11 Thread David M. Lloyd

On 12/11/2016 05:18 AM, Alan Bateman wrote:

On 10/12/2016 15:21, David M. Lloyd wrote:


Would it be possible to have all available selector provider
implementation classes listed in a "provides java.nio.channels.spi
with ..." section of java.base's module descriptor?  My use case is as
follows:

Our I/O library (XNIO) relies on the ability to detect and use
different available selector providers for different purposes in
different circumstances (often to provide alternatives in the event of
platform-specific behavior problems, or to prefer lighter providers
over heavier ones in certain situations).  Right now we directly use
reflection to seek out specific named classes. However, it would be
much better if we could instead use a service loader to discover all
available implementations, which would free us from having to use
reflection for this purpose and also avoid lots of pointless probing.

Today you need to have --add-exports=java.base/sun.nio.ch=xxx which is
definitely not ideal.

Maybe there's another solution to this problem as well; suggestions
welcome.


I don't think we've ever come across alternative implementations of
SelectorProvider, are these complete implementations?


I'm not implementing SelectorProvider, I just want access to all the 
possible implementations that are available in the current JDK so we can 
select among them on a case-by-case basis.



Are you attempting
to wrap the default implementation - I ask because there isn't anything
in the service type (SelectorProvider in this example) that is useful
for doing selection.


Except for the name of the class, which is what we select by today.
--
- DML


Re: Java 9 build 148 causes trouble in Apache Lucene/Solr/Elasticsearch

2016-12-11 Thread Carsten Varming
Dear Alan,

On Sun, Dec 11, 2016 at 6:16 AM, Alan Bateman 
wrote:

>
> The alternative is of course:
>
> ByteBuffer wrap(long address, int capacity)
> void unmap(MappedByteBuffer)
>
> The wrap method allow be similar to JNI's NewDirectByteBuffer for those
> that are managing the underlying memory themselves. This makes it a more
> advanced method to avoid too much temptation to free the memory underlying
> a buffer created with ByteBuffer.allocateDirect. We can't do much with
> unmap but that at least won't be widely used.


I previously patched Netty to use the Runnable cleaner, so I have some
interesting in this discussion.

Having a public method "ByteBuffer wrap(long adddress, int capacity) in the
standard would simplify Netty code. Netty currently use the cleaner on
ByteBuffers allocated by ByteBuffer.allocateDirect, but I believe that can
be changed.

Carsten


Re: Java 9 build 148 causes trouble in Apache Lucene/Solr/Elasticsearch

2016-12-11 Thread Peter Levart



On 12/11/2016 09:59 PM, Peter Levart wrote:

The alternative is of course:

ByteBuffer wrap(long address, int capacity)
void unmap(MappedByteBuffer)

The wrap method allow be similar to JNI's NewDirectByteBuffer for 
those that are managing the underlying memory themselves. This makes 
it a more advanced method to avoid too much temptation to free the 
memory underlying a buffer created with ByteBuffer.allocateDirect. We 
can't do much with unmap but that at least won't be widely used.


-Alan.


So, If I understand correctly, you are proposing users to do their own 
memory allocation/management/deallocation with Unsafe and use wrap() 
just to create a view of the allocated memory in the form of 
ByteBuffer. Wouldn't this force them to use a different approach to 
managing ByteBuffer(s) from what they do now - they would have to keep 
the allocated memory's address somewhere outside the buffer in order 
to free the memory later... It is doable, but I think Uwe will not 
like it very much.


Sorry, I just realized that Uwe is interested only in the unmapping case...



About unmap()... Is it just the name and signature that would 
discourage freeing memory underlying a buffer created with 
ByteBuffer.allocateDirect() or do you propose to disallow such use and 
only allow actual unmapping?




Re: Java 9 build 148 causes trouble in Apache Lucene/Solr/Elasticsearch

2016-12-11 Thread Peter Levart

Hi Alan,


On 12/11/2016 12:16 PM, Alan Bateman wrote:



On 10/12/2016 17:11, Chris Hegarty wrote:

:

How about: Unsafe::deallocate(ByteBuffer directBuffer)?
   http://cr.openjdk.java.net/~chegar/Unsafe_deallocate/


The alternative is of course:

ByteBuffer wrap(long address, int capacity)
void unmap(MappedByteBuffer)

The wrap method allow be similar to JNI's NewDirectByteBuffer for 
those that are managing the underlying memory themselves. This makes 
it a more advanced method to avoid too much temptation to free the 
memory underlying a buffer created with ByteBuffer.allocateDirect. We 
can't do much with unmap but that at least won't be widely used.


-Alan.


So, If I understand correctly, you are proposing users to do their own 
memory allocation/management/deallocation with Unsafe and use wrap() 
just to create a view of the allocated memory in the form of ByteBuffer. 
Wouldn't this force them to use a different approach to managing 
ByteBuffer(s) from what they do now - they would have to keep the 
allocated memory's address somewhere outside the buffer in order to free 
the memory later... It is doable, but I think Uwe will not like it very 
much.


About unmap()... Is it just the name and signature that would discourage 
freeing memory underlying a buffer created with 
ByteBuffer.allocateDirect() or do you propose to disallow such use and 
only allow actual unmapping?


Regards, Peter



Re: Java 9 build 148 causes trouble in Apache Lucene/Solr/Elasticsearch

2016-12-11 Thread Peter Levart

Hi Uwe,


On 12/11/2016 01:57 PM, Uwe Schindler wrote:


Hi,

How about the following:

-Check that the buffer is direct, if not throw IAE(“not direct buffer”)

-Check that buffer has attachment==null (this tells you that it’s not 
a slice/dup), if not throw IAE(“not allowed to free duplicates/slices”)


-Finally do the standard if (cleaner!=null) cleaner.clean(), but don’t 
throw any exceptions if cleaner is null (as this is implementation detail)


This allows for empty buffers without cleaner that are still marked as 
direct. But it disallows all slices or duplicates.




Yes, this would be the right logic I agree. It would silently ignore the 
requests to free memory for buffers constructed via JNI's 
NewDirectByteBuffer calls, but I suppose this would not be a problem in 
practice.


I am fine with Alan’s proposal to restrict to MappedByteBuffer but 
that’s out of my interest – I am happy to unmap mapped byte buffers. I 
would also place the method in the legacy sun.misc.Unsafe only, the 
JDK-internal private one is not accessible to the outside. Of course 
for consistency it could be in both, but primarily it must be in 
sun.misc.Unsafe – that’s also what most code is using anyways.




Yes, internally, at least in java.base, the code can always directly 
invoke the DirectBuffer's and Cleaner's methods...


Regards, Peter



Uwe

*From:*Peter Levart [mailto:peter.lev...@gmail.com]
*Sent:* Saturday, December 10, 2016 10:23 PM
*To:* Uwe Schindler ; Chris Hegarty 

*Cc:* jigsaw-dev@openjdk.java.net; Core-Libs-Dev 

*Subject:* Re: Java 9 build 148 causes trouble in Apache 
Lucene/Solr/Elasticsearch


On 12/10/2016 09:00 PM, Uwe Schindler wrote:

Hi,

We noticed that buffers with zero length also have no cleaner.
This is why we also have the null check in our code (see Github)
and the guardWithTest in the MethodHandle, although we never free
duplicates. So a noop is better imho.


Oh yes, good catch. Then what about being noop just for zero length? I 
don't know, maybe I'm just being paranoid and those who would use this 
API know perfectly well what they are doing. I'm just imagining a 
situation where one would create and keep just a duplicate of a direct 
buffer and afterwards use it to try to deallocate the native memory. 
This would be a noop, but the developer would think it works as GC 
would finally do it for him. I think it's better to throw an exception 
to prevent such situations...


Regards, Peter



I like the Unsafe approach. To me both variants are fine.

Uwe

Am 10. Dezember 2016 20:47:46 MEZ schrieb Peter Levart
 :

Hi Chris,

On 12/10/2016 06:11 PM, Chris Hegarty wrote:

How about: Unsafe::deallocate(ByteBuffer directBuffer)?

   http://cr.openjdk.java.net/~chegar/Unsafe_deallocate/



Apart from the fact that Unsafe is (was?) reserved for
low-level stuff, I think this approach is reasonable. Is the
method in jdk.internal.misc.Unsafe needed? You could add the
method just to the sun.misc.Unsafe (to keep internal Unsafe
free from hacks) and export the two packages selectively to
jdk.unsupported.


We could attempt to limit this to the direct buffer that "owns" the

memory, i.e. not a duplicate or a slice, but I'm not sure it is 
worth

it.


What you have here *is* limited to direct ByteBuffer(s) that
"own" the memory. Derived buffer(s) (duplicated or sliced) do
not have a Cleaner instance (they have an 'attachment' to keep
the 1st-level buffer reachable while they are reachable). I
would even make it more unforgiving by throwing an IAE if the
passed-in buffer didn't have a Cleaner. In addition I would
specify this behavior. For example:

"Deallocates the underlying memory associated with given
directBuffer if the buffer was obtained from either {@link
ByteBuffer#allocateDirect} or {@link FileChannel#map} methods.
In any other case (when the buffer is not a direct buffer or
was obtained by  {@link ByteBuffer#duplicate() duplicating} or
{@link ByteBuffer#slice(int, int) slicing} a direct buffer),
the method throws {@code IllegalArgumentException}.

Regards, Peter





Re: Java 9 build 148 causes trouble in Apache Lucene/Solr/Elasticsearch

2016-12-11 Thread Peter Levart

Hi Chris,


On 12/11/2016 10:26 AM, Chris Hegarty wrote:

>"Deallocates the underlying memory associated with given directBuffer if the 
buffer was obtained from either {@link ByteBuffer#allocateDirect} or {@link 
FileChannel#map} methods. In any other case (when the buffer is not a direct buffer or 
was obtained by  {@link ByteBuffer#duplicate() duplicating} or {@link 
ByteBuffer#slice(int, int) slicing} a direct buffer), the method throws {@code 
IllegalArgumentException}.

Yes, but given a ByteBuffer it is not possible to determine if it “owns” the
memory, or not. So users of the API would have to have full knowledge of
the buffers they pass to it. Maybe this is ok?

-Chris.


In order for deallocation to be effective and, above all, safe, user has 
to know the whole story of a buffer (s)he intends to deallocate and the 
story of all possible derived buffers. I don't believe one can create a 
safe program by choosing to deallocate a direct buffer for which (s)he 
does not know where it came from, because then (s)he also doesn't know 
what other buffers might still be using the same piece of memory.


Regards, Peter



Re: Review Request JDK-8169925: Organize licenses by module in source, JMOD file, and run-time image

2016-12-11 Thread Philip Race

The parts of this which I know about (the client licenses) look fine to me.
Some day we should look at whether we still have vestiges of X11
in the Mac code from the BSD port but for now its safer to assume there 
are ..
By this I mean "unix" includes Mac. as a core OS, but not as a desktop 
technology

for our port.

-phil.

On 12/7/16, 1:28 PM, Mandy Chung wrote:

This proposes to organize license files by module in source, JMOD,
and run-time image.

A summary of the proposal:
1. Organize third party notices by module in the source as follows:
 src/$MODULE/{share,$OS}/legal/*

The `legal` directory contains one file for each third party
library in the module, for example,
 src/java.base/share/legal/asm.md
   unicode.md
   zlib.md

The proposed template for this file is described in [1] and JEP 201
will be updated to reflect this proposed source layout.

2. Introduce a new LEGAL_NOTICES section in JMOD format. A new jmod
option `—-legal-notices` is added to package legal notices in
a JMOD file.

3. At jlink time, jlink will copy all legal notices from JMOD files
to the `legal` directory in the run-time image.  A plugin is
added to de-duplicate the legal notices if the filename and the
content matches that may reduce the image footprint.

4. THIRD_PARTY_README in the top-level directory of each repo is removed.
Manual edit to this file, multiple copies is no longer needed.

Webrev at:
http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8169925/webrev.00/

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


Question ad #AwkwardStrongEncapsulation (Re: Moving the changes in jake to jdk9/dev

2016-12-11 Thread Rony G. Flatscher
On 23.11.2016 12:55, Alan Bateman wrote:
>
> We've been accumulating changes in the jake forest that are tied to JSR 
> issues for the last few
> months. Some of the changes (such as #ClassLoaderNames) have already been 
> pushed upstream to
> jdk9/dev but we've still sitting on a large patch.
>
> We would like to move the changes that we have in jake to jdk9/dev soon, 
> early December if
> possible. One motivation is to get the changes and new APIs into JDK 9 main 
> line so that they can
> be used and tested more widely. This should help reduce some of the confusion 
> with having two
> builds too. Another motivation is that the merging is getting painful, esp. 
> the langtools
> repository where we have regular conflicts with changes to javac pushed via 
> jdk9/dev.
>
> To that end, I will start a code review soon that will be a snapshot of the 
> changes in the jake
> forest. Once these changes are reviewed and in jdk9/dev then we will continue 
> in the jake forest
> where appropriate - particularly in areas that are tied to JSR issues where 
> we need to iterate and
> prototype, also in areas where we need to re-work some areas of the 
> implementation.
>
> As people on this mailing list know, jake has the changes for 
> #AwkwardStrongEncapsulation [1]
> where setAccessible has been changed so it can't be used to break into 
> non-public members/types in
> exported packages of JDK modules. It was changed more than a year ago to fail 
> when attempting to
> use it to break into non-exported packages. Dialing it up further is a 
> disruptive change that will
> expose a lot of hacks and issues with existing code that is used to accessing 
> non-public
> fields/methods in JDK classes. It will take some libraries and tools a bit of 
> time to digest this
> change, even with the --add-opens command line option and Add-Opens manifest 
> in application JAR
> files to keep existing code going. I plan to send mail to jdk9-dev in advance 
> of this integration
> to create wider awareness of this change.
>
> -Alan
>
> [1] 
> http://openjdk.java.net/projects/jigsaw/spec/issues/#AwkwardStrongEncapsulation
Would #AwkwardStrongEncapsulation inhibit setAccessible to work on protected 
methods (in addition to
private and package private members) as well?

As subclasses are allowed to access protected members in their superclasses, 
setAccessible should
work for protected methods in classes that are invoked for objects that are 
instances of their
subclasses?

---rony






Re: Module exports for java.base: NIO stuff

2016-12-11 Thread Alan Bateman

On 11/12/2016 11:44, Remi Forax wrote:


:
No, these are different implementation, they directly use classes from 
sun.nio.ch and bypass the selector API.

Sure, but the SelectorProvider service interface doesn't define anything 
that allows someone to choose when there are multiple implementations 
deployed. I compare that to FileSystemProvider where you choose based on 
the URI scheme.


-Alan


Re: Sync with jdk9 and ASM alpha 2

2016-12-11 Thread Alan Bateman

On 11/12/2016 13:05, Remi Forax wrote:


Hi Alan,
if i get it right, the jdk9 b148 (not the jigsaw version) get a refresh of 
module-info classfile format but did not get the last two changes,
version of requires and the two new constant pool entries.
Do you plan to push these changes in the jdk9 workspace soon so i will delay 
the release of ASM 6 alpha 2 or do you plan to introduce these changes later so 
i should release an alpha 2 now and release an alpha 3/beta later ?


That's right. We are going to aim for jdk-9+150 to get the latest 
proposals integrated.


-Alan


RE: Java 9 build 148 causes trouble in Apache Lucene/Solr/Elasticsearch

2016-12-11 Thread Uwe Schindler
Hi,

How about the following:

 

-  Check that the buffer is direct, if not throw IAE(“not direct 
buffer”)

-  Check that buffer has attachment==null (this tells you that it’s not 
a slice/dup), if not throw IAE(“not allowed to free duplicates/slices”)

-  Finally do the standard if (cleaner!=null) cleaner.clean(), but 
don’t throw any exceptions if cleaner is null (as this is implementation detail)

 

This allows for empty buffers without cleaner that are still marked as direct. 
But it disallows all slices or duplicates. 

 

I am fine with Alan’s proposal to restrict to MappedByteBuffer but that’s out 
of my interest – I am happy to unmap mapped byte buffers. I would also place 
the method in the legacy sun.misc.Unsafe only, the JDK-internal private one is 
not accessible to the outside. Of course for consistency it could be in both, 
but primarily it must be in sun.misc.Unsafe – that’s also what most code is 
using anyways.

 

Uwe

 

 

From: Peter Levart [mailto:peter.lev...@gmail.com] 
Sent: Saturday, December 10, 2016 10:23 PM
To: Uwe Schindler ; Chris Hegarty 

Cc: jigsaw-dev@openjdk.java.net; Core-Libs-Dev 
Subject: Re: Java 9 build 148 causes trouble in Apache Lucene/Solr/Elasticsearch

 

 

 

On 12/10/2016 09:00 PM, Uwe Schindler wrote:

Hi,

We noticed that buffers with zero length also have no cleaner. This is why we 
also have the null check in our code (see Github) and the guardWithTest in the 
MethodHandle, although we never free duplicates. So a noop is better imho.


Oh yes, good catch. Then what about being noop just for zero length? I don't 
know, maybe I'm just being paranoid and those who would use this API know 
perfectly well what they are doing. I'm just imagining a situation where one 
would create and keep just a duplicate of a direct buffer and afterwards use it 
to try to deallocate the native memory. This would be a noop, but the developer 
would think it works as GC would finally do it for him. I think it's better to 
throw an exception to prevent such situations...

Regards, Peter





I like the Unsafe approach. To me both variants are fine.

Uwe

Am 10. Dezember 2016 20:47:46 MEZ schrieb Peter Levart  
 : 

Hi Chris,

 

On 12/10/2016 06:11 PM, Chris Hegarty wrote:

How about: Unsafe::deallocate(ByteBuffer directBuffer)?
  http://cr.openjdk.java.net/~chegar/Unsafe_deallocate/ 
 


Apart from the fact that Unsafe is (was?) reserved for low-level stuff, I think 
this approach is reasonable. Is the method in jdk.internal.misc.Unsafe needed? 
You could add the method just to the sun.misc.Unsafe (to keep internal Unsafe 
free from hacks) and export the two packages selectively to jdk.unsupported.
 



We could attempt to limit this to the direct buffer that "owns" the
memory, i.e. not a duplicate or a slice, but I'm not sure it is worth
it.


What you have here *is* limited to direct ByteBuffer(s) that "own" the memory. 
Derived buffer(s) (duplicated or sliced) do not have a Cleaner instance (they 
have an 'attachment' to keep the 1st-level buffer reachable while they are 
reachable). I would even make it more unforgiving by throwing an IAE if the 
passed-in buffer didn't have a Cleaner. In addition I would specify this 
behavior. For example:

"Deallocates the underlying memory associated with given directBuffer if the 
buffer was obtained from either {@link ByteBuffer#allocateDirect} or {@link 
FileChannel#map} methods. In any other case (when the buffer is not a direct 
buffer or was obtained by  {@link ByteBuffer#duplicate() duplicating} or {@link 
ByteBuffer#slice(int, int) slicing} a direct buffer), the method throws {@code 
IllegalArgumentException}.

Regards, Peter

 



Re: Module exports for java.base: NIO stuff

2016-12-11 Thread Remi Forax
- Mail original -
> De: "Alan Bateman" 
> À: "David M. Lloyd" , "jigsaw-dev" 
> 
> Envoyé: Dimanche 11 Décembre 2016 12:18:26
> Objet: Re: Module exports for java.base: NIO stuff

> On 10/12/2016 15:21, David M. Lloyd wrote:
> 
>> Would it be possible to have all available selector provider
>> implementation classes listed in a "provides java.nio.channels.spi
>> with ..." section of java.base's module descriptor?  My use case is as
>> follows:
>>
>> Our I/O library (XNIO) relies on the ability to detect and use
>> different available selector providers for different purposes in
>> different circumstances (often to provide alternatives in the event of
>> platform-specific behavior problems, or to prefer lighter providers
>> over heavier ones in certain situations).  Right now we directly use
>> reflection to seek out specific named classes. However, it would be
>> much better if we could instead use a service loader to discover all
>> available implementations, which would free us from having to use
>> reflection for this purpose and also avoid lots of pointless probing.
>>
>> Today you need to have --add-exports=java.base/sun.nio.ch=xxx which is
>> definitely not ideal.
>>
>> Maybe there's another solution to this problem as well; suggestions
>> welcome.
>>
> I don't think we've ever come across alternative implementations of
> SelectorProvider, are these complete implementations? Are you attempting
> to wrap the default implementation - I ask because there isn't anything
> in the service type (SelectorProvider in this example) that is useful
> for doing selection.

No, these are different implementation, they directly use classes from 
sun.nio.ch and bypass the selector API.

> 
> -Alan.

Rémi


Re: jlink and automatic modules

2016-12-11 Thread Alan Bateman

On 10/12/2016 22:40, Claes Redestad wrote:



The intended behavior is for jlink to operate on any set of modules to
support linking images for other platforms, operating systems etc (the
java.base jmod shipped with any JDK contains a native JVM), so it may
feel more natural to make it explicit all the way rather than design a
way to exclude and override parts of the module-path.

I see no obvious technical reason it wouldn't work if someone put their
mind to it, but it might be a non-trivial effort to get right for
something that's really only a small quality of life improvement.
Sundar has had one or two proposals on this. As you note, the solution 
needs to take account of cross linking and also the scenario where the 
jlink tool is running on one JDK but is producing a run time image from 
packages modules for a different JDK (say a tool running on 9 but 
producing a run-time image for 9.1).


-Alan


Re: Module exports for java.base: NIO stuff

2016-12-11 Thread Alan Bateman

On 10/12/2016 15:21, David M. Lloyd wrote:

Would it be possible to have all available selector provider 
implementation classes listed in a "provides java.nio.channels.spi 
with ..." section of java.base's module descriptor?  My use case is as 
follows:


Our I/O library (XNIO) relies on the ability to detect and use 
different available selector providers for different purposes in 
different circumstances (often to provide alternatives in the event of 
platform-specific behavior problems, or to prefer lighter providers 
over heavier ones in certain situations).  Right now we directly use 
reflection to seek out specific named classes. However, it would be 
much better if we could instead use a service loader to discover all 
available implementations, which would free us from having to use 
reflection for this purpose and also avoid lots of pointless probing.


Today you need to have --add-exports=java.base/sun.nio.ch=xxx which is 
definitely not ideal.


Maybe there's another solution to this problem as well; suggestions 
welcome.


I don't think we've ever come across alternative implementations of 
SelectorProvider, are these complete implementations? Are you attempting 
to wrap the default implementation - I ask because there isn't anything 
in the service type (SelectorProvider in this example) that is useful 
for doing selection.


-Alan.



Re: Java 9 build 148 causes trouble in Apache Lucene/Solr/Elasticsearch

2016-12-11 Thread Alan Bateman



On 10/12/2016 17:11, Chris Hegarty wrote:

:

How about: Unsafe::deallocate(ByteBuffer directBuffer)?
   http://cr.openjdk.java.net/~chegar/Unsafe_deallocate/


The alternative is of course:

ByteBuffer wrap(long address, int capacity)
void unmap(MappedByteBuffer)

The wrap method allow be similar to JNI's NewDirectByteBuffer for those 
that are managing the underlying memory themselves. This makes it a more 
advanced method to avoid too much temptation to free the memory 
underlying a buffer created with ByteBuffer.allocateDirect. We can't do 
much with unmap but that at least won't be widely used.


-Alan.


Re: Java 9 build 148 causes trouble in Apache Lucene/Solr/Elasticsearch

2016-12-11 Thread Chris Hegarty

> On 10 Dec 2016, at 19:47, Peter Levart  wrote:
> 
> Hi Chris,
> 
> On 12/10/2016 06:11 PM, Chris Hegarty wrote:
>> How about: Unsafe::deallocate(ByteBuffer directBuffer)?
>>   
>> http://cr.openjdk.java.net/~chegar/Unsafe_deallocate/
> 
> Apart from the fact that Unsafe is (was?) reserved for low-level stuff, I 
> think this approach is reasonable. Is the method in jdk.internal.misc.Unsafe 
> needed? You could add the method just to the sun.misc.Unsafe (to keep 
> internal Unsafe free from hacks) and export the two packages selectively to 
> jdk.unsupported.

Yes, possibly.

>> We could attempt to limit this to the direct buffer that "owns" the
>> memory, i.e. not a duplicate or a slice, but I'm not sure it is worth
>> it.
>> 
> 
> What you have here *is* limited to direct ByteBuffer(s) that "own" the memory.

Understood, what I meant was throwing an exception if the given buffer
does not “own” the memory.

> Derived buffer(s) (duplicated or sliced) do not have a Cleaner instance (they 
> have an 'attachment' to keep the 1st-level buffer reachable while they are 
> reachable). I would even make it more unforgiving by throwing an IAE if the 
> passed-in buffer didn't have a Cleaner. In addition I would specify this 
> behavior. For example:
> 
> "Deallocates the underlying memory associated with given directBuffer if the 
> buffer was obtained from either {@link ByteBuffer#allocateDirect} or {@link 
> FileChannel#map} methods. In any other case (when the buffer is not a direct 
> buffer or was obtained by  {@link ByteBuffer#duplicate() duplicating} or 
> {@link ByteBuffer#slice(int, int) slicing} a direct buffer), the method 
> throws {@code IllegalArgumentException}.

Yes, but given a ByteBuffer it is not possible to determine if it “owns” the
memory, or not. So users of the API would have to have full knowledge of
the buffers they pass to it. Maybe this is ok?

-Chris.