Re: RFR: 8003322: Add instrumentation points for tracing of I/O calls

2012-11-26 Thread Staffan Larsen
A webrev for the changes going into 7u12 is here: 
http://cr.openjdk.java.net/~sla/8003322/webrev.jdk7.00/

Thanks,
/Staffan

On 20 nov 2012, at 18:30, Mandy Chung  wrote:

> On 11/20/12 6:20 AM, Alan Bateman wrote:
>> On 20/11/2012 14:10, Staffan Larsen wrote:
>>> :
>>> The original plan was for the code in this review to go into both 8 and 
>>> 7u12. Since 7u12 has a tighter deadline, a possible path would be to 
>>> include it only in 7u12, but not in 8. For 8 we would then implement a 
>>> fully dynamic solution. The only thing needed in 8 from this review would 
>>> be the path field added to the stream classes. Does that sound like a plan?
>>> 
>> I think this is a good plan. I suggest bringing it up on jdk7u-dev so that 
>> the jdk7u maintainers can think about the issue (rather that trying to get 
>> approval at the last minute when you are reading to push).
> 
> This sounds a good plan to me too.  This would take the pressure off 7u12 
> deadline while a better solution will be implemented for 8.
> 
> Mandy



Re: Review request: JDK-7162111 TEST_BUG: change tests run in headless mode [macosx] (open part - ver. 3)

2012-11-26 Thread Alexey Utkin

I need the second reviewer.

-uta

On 23.11.2012 15:57, Alan Bateman wrote:

On 23/11/2012 11:40, Alexey Utkin wrote:

I mean:
  # jdk_io
-# 7162111 - these tests need to be updated to run headless
-java/io/Serializable/resolveClass/deserializeButton/run.sh  macosx-all
-java/io/Serializable/serialver/classpath/run.sh macosx-all
-java/io/Serializable/serialver/nested/run.shmacosx-all
-
  

Yes, exactly.

-Alan




Re: Review request: JDK-7162111 TEST_BUG: change tests run in headless mode [macosx] (open part - ver. 3)

2012-11-26 Thread Alan Bateman

On 26/11/2012 10:38, Alexey Utkin wrote:

I need the second reviewer.

-uta

One reviewer is fine.

-Alan


hg: jdk8/tl/jdk: 7162111: TEST_BUG: change tests run in headless mode [macosx] (open)

2012-11-26 Thread alexey . utkin
Changeset: 8970128e040d
Author:uta
Date:  2012-11-26 15:54 +0400
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/8970128e040d

7162111: TEST_BUG: change tests run in headless mode [macosx] (open)
Summary: In problem tests detection of AWT headless mode was introduced or AWT 
dependence was removed.
Reviewed-by: alanb

! test/ProblemList.txt
! test/demo/jvmti/mtrace/TraceJFrame.java
! test/java/io/Serializable/resolveClass/deserializeButton/Foo.java
! test/java/io/Serializable/resolveClass/deserializeButton/Test.java
! test/java/io/Serializable/resolveClass/deserializeButton/run.sh



Re: RFR: 8003322: Add instrumentation points for tracing of I/O calls

2012-11-26 Thread Alan Bateman

On 26/11/2012 09:40, Staffan Larsen wrote:
A webrev for the changes going into 7u12 is here: 
http://cr.openjdk.java.net/~sla/8003322/webrev.jdk7.00/ 



I think the changes are fine for 7u12 and we look forward to a less 
invasive solution for jdk8.


-Alan


Re: 8003949: LogManager, downgrade normative reference to ${java.home}/lib/logging.properties

2012-11-26 Thread Paul Sandoz
Hi Alan,

+ * If neither of these properties is defined then the LogManager uses its
+ * default configuration. The default configuration is typically loaded from 
the
+ * properties file "{@code lib/logging.properties}" in the Java installation 
+ * directory.

Will typical become atypical for OpenJDK by the time Java 9 ships?

Paul.

On Nov 25, 2012, at 11:07 PM, Alan Bateman  wrote:

> 
> As part of preparing for modules [1], we need to examine a number of 
> normative references to files in ${java.home} with a view to downgrading 
> these references to non-normative status. This is important as we need the 
> flexibility to eventually move some of these files into module-private 
> locations, maybe in some cases replace them with something else. This is 
> something I've bought up on security-dev and i18n-dev recently as there are 
> number of references to files in ${java.home} that need to be examined.
> 
> The focus of this mail is java.util.logging.LogManager as it specifies that 
> the default configuration is loaded from ${java.home}/lib/logging.properties. 
> Clearly this file is changed in some environments, although running with 
> java.util.logging.config.file is probably more robust in that the settings 
> can be used with different JDK installations. The proposed changes (javadoc 
> changes only, no implementation changes) is here:
>  http://cr.openjdk.java.net/~alanb/8003949/webrev/
> 
> Note that I have also removed the statement that "properties may be set via 
> the Preference API" as the implementation has never used the preferences and 
> not worth re-visiting now.
> 
> -Alan
> 
> [1] http://openjdk.java.net/jeps/162



Re: 8003949: LogManager, downgrade normative reference to ${java.home}/lib/logging.properties

2012-11-26 Thread Alan Bateman

On 26/11/2012 15:18, Paul Sandoz wrote:

Hi Alan,

+ * If neither of these properties is defined then the LogManager uses its
+ * default configuration. The default configuration is typically loaded from 
the
+ * properties file "{@code lib/logging.properties}" in the Java installation
+ * directory.

Will typical become atypical for OpenJDK by the time Java 9 ships?

It's possible although in the current jigsaw/jigsaw builds we have 
retained this and other files that users might change in their original 
location. The javadoc change, along with a few others in the pipe, will 
lesson the impact of moving to a modules image a module-private 
configuration.


-Alan.


Re: 8003949: LogManager, downgrade normative reference to ${java.home}/lib/logging.properties

2012-11-26 Thread Mandy Chung

On 11/25/12 2:07 PM, Alan Bateman wrote:


...

The focus of this mail is java.util.logging.LogManager as it specifies 
that the default configuration is loaded from 
${java.home}/lib/logging.properties. Clearly this file is changed in 
some environments, although running with java.util.logging.config.file 
is probably more robust in that the settings can be used with 
different JDK installations. The proposed changes (javadoc changes 
only, no implementation changes) is here:

  http://cr.openjdk.java.net/~alanb/8003949/webrev/



This looks fine with me.

Note that I have also removed the statement that "properties may be 
set via the Preference API" as the implementation has never used the 
preferences and not worth re-visiting now.




I agree that it's good to take this statement out.

Mandy


Re: RFR: 8003322: Add instrumentation points for tracing of I/O calls

2012-11-26 Thread Mandy Chung

On 11/26/12 1:40 AM, Staffan Larsen wrote:
A webrev for the changes going into 7u12 is here: 
http://cr.openjdk.java.net/~sla/8003322/webrev.jdk7.00/ 



The changes look fine for 7u12.Thanks for looking into the dynamic 
bytecode instrumentation solution for jdk8.


Mandy


Re: Adding field to BatchUpdateException

2012-11-26 Thread Lance Andersen - Oracle
Hi Joe,

Thank you for the sanity check.

I had added the following to the top of the javadoc (still playing with the 
wording):

As of Java SE 8, the method getLargeUpdateCount has been added to provide 
support for update counts that may be exceed Integer.MAX_VALUE and returned by 
the method Statement.executeLargeBatch. A JDBC driver implementation is 
required to throw BatchUpdateException(String reason, String SQLState, int 
vendorCode, long []updateCounts,Throwable cause) if an error occurs during the 
the execution of Statement.executeLargeBatch. If Statement.executeLargeBatch is 
invoked it is recommended that getLargeUpdateCounts be called instead of 
getUpdateCounts in order to avoid a possible overflow of the integer update 
count.


Best
Lance

On Nov 26, 2012, at 1:51 AM, Joe Darcy wrote:

> Hi Lance,
> 
> I don't see an obvious problem with the code, but I strongly suggest 
> documenting the correctness conditions regarding the updateCounts and 
> longUpdateCounts fields; I think that would ease reviewing the new 
> constructors and serialization code.
> 
> Cheers,
> 
> -Joe
> 
> On 11/24/2012 2:05 PM, Lance Andersen - Oracle wrote:
>> Hi,
>> 
>> For JDBC 4.2, I am adding methods to allow for larger update counts (request 
>> from JDBC driver vendors)  and because of this I have to tweak 
>> BatchUpdateException
>> 
>> The Statement interface has the method
>> 
>> int[] executeBatch()
>> 
>> I am planning to add
>> 
>> long[] executeLargeBatch().
>> 
>> To accomodate this change, I  also need to add a new field and the method 
>> getLargeUpdateCount to BatchUpdateException.
>> 
>> I have exchanged emails on this with Alan and he indicated that the changes 
>> seemed reasonable but to send a general email out to see if anything was  
>> missed from the serialization perspective.
>> 
>> I have added JDBC Unit tests to validate that the 
>> serialization/deserialization works between JDBC 4.1 and JDBC 4.2 and they 
>> run without a problem.
>> 
>> 
>> Best
>> Lance
>> 
>> new-host-2:sql lanceandersen$ diff BatchUpdateException.java 
>> ~/NetBeansProjects/JDBC4.2/jdbc4.0/src/java/sql/
>> 2c2
>> <  * Copyright (c) 1998, 2011, Oracle and/or its affiliates. All rights 
>> reserved.
>> ---
>>>  * Copyright (c) 1998, 2012, Oracle and/or its affiliates. All rights 
>>> reserved.
>> 27a28,31
>>> import java.io.IOException;
>>> import java.io.InvalidObjectException;
>>> import java.io.ObjectInputStream;
>>> import java.io.ObjectOutputStream;
>> 83a88
>>>   this.longUpdateCounts = (updateCounts == null) ? null : 
>>> copyUpdateCount(updateCounts);
>> 192c197
>> < this((cause == null ? null : cause.toString()), null, 0, null, 
>> cause);
>> ---
>>> this((cause == null ? null : cause.toString()), null, 0, 
>>> (int[])null, cause);
>> 295a301
>>> this.longUpdateCounts = (updateCounts == null) ? null : 
>>> copyUpdateCount(updateCounts);
>> 331c337,401
>> <
>> ---
>>>/**
>>>* Constructs a BatchUpdateException object initialized with
>>>* a given reason, SQLState, 
>>> vendorCode
>>>* cause and updateCounts.
>>>* 
>>>* This constructor should be used when the returned update count may 
>>> exceed
>>>* {@link Integer.MAX_VALUE}.
>>>* 
>>>* @param reason a description of the error
>>>* @param SQLState an XOPEN or SQL:2003 code identifying the exception
>>>* @param vendorCode an exception code used by a particular
>>>* database vendor
>>>* @param updateCounts an array of long, with each element
>>>*indicating the update count, Statement.SUCCESS_NO_INFO or
>>>* Statement.EXECUTE_FAILED for each SQL command in
>>>* the batch for JDBC drivers that continue processing
>>>* after a command failure; an update count or
>>>* Statement.SUCCESS_NO_INFO for each SQL command in the 
>>> batch
>>>* prior to the failure for JDBC drivers that stop processing after a 
>>> command
>>>* failure
>>>* @param cause the underlying reason for this SQLException
>>>* (which is saved for later retrieval by the getCause() 
>>> method);
>>>* may be null indicating the cause is non-existent or unknown.
>>>* @since 1.8
>>>*/
>>>   public BatchUpdateException(String reason, String SQLState, int 
>>> vendorCode,
>>>   long []updateCounts,Throwable cause) {
>>>   super(reason, SQLState, vendorCode, cause);
>>>   this.longUpdateCounts  = (updateCounts == null) ? null : 
>>> Arrays.copyOf(updateCounts, updateCounts.length);
>>>   this.updateCounts = (longUpdateCounts == null) ? null : 
>>> copyUpdateCount(longUpdateCounts);
>>>   }
>>> /**
>>>* Retrieves the update count for each update statement in the batch
>>>* update that executed successfully before this exception occurred.
>>>* A driver that implements batch updates may or may not continue to
>>>* process the remaining commands in a batch when one of the commands
>>>* fails to execute properly. If the driver continues 

Re: Adding field to BatchUpdateException

2012-11-26 Thread Ulf Zibis

Hi Lance,

I would throw an IllegalStateException if invoking e.g. getUpdateCounts on 
integer overflow.

-Ulf


Am 26.11.2012 20:44, schrieb Lance Andersen - Oracle:

Hi Joe,

Thank you for the sanity check.

I had added the following to the top of the javadoc (still playing with the 
wording):

As of Java SE 8, the method getLargeUpdateCount has been added to provide 
support for update counts that may be exceed Integer.MAX_VALUE and returned by 
the method Statement.executeLargeBatch. A JDBC driver implementation is 
required to throw BatchUpdateException(String reason, String SQLState, int 
vendorCode, long []updateCounts,Throwable cause) if an error occurs during the 
the execution of Statement.executeLargeBatch. If Statement.executeLargeBatch is 
invoked it is recommended that getLargeUpdateCounts be called instead of 
getUpdateCounts in order to avoid a possible overflow of the integer update 
count.


Best
Lance

On Nov 26, 2012, at 1:51 AM, Joe Darcy wrote:


Hi Lance,

I don't see an obvious problem with the code, but I strongly suggest 
documenting the correctness conditions regarding the updateCounts and 
longUpdateCounts fields; I think that would ease reviewing the new constructors 
and serialization code.

Cheers,

-Joe

On 11/24/2012 2:05 PM, Lance Andersen - Oracle wrote:

Hi,

For JDBC 4.2, I am adding methods to allow for larger update counts (request 
from JDBC driver vendors)  and because of this I have to tweak 
BatchUpdateException

The Statement interface has the method

int[] executeBatch()

I am planning to add

long[] executeLargeBatch().

To accomodate this change, I  also need to add a new field and the method 
getLargeUpdateCount to BatchUpdateException.

I have exchanged emails on this with Alan and he indicated that the changes 
seemed reasonable but to send a general email out to see if anything was  
missed from the serialization perspective.

I have added JDBC Unit tests to validate that the serialization/deserialization 
works between JDBC 4.1 and JDBC 4.2 and they run without a problem.


Best
Lance

new-host-2:sql lanceandersen$ diff BatchUpdateException.java 
~/NetBeansProjects/JDBC4.2/jdbc4.0/src/java/sql/
2c2
<  * Copyright (c) 1998, 2011, Oracle and/or its affiliates. All rights 
reserved.
---

  * Copyright (c) 1998, 2012, Oracle and/or its affiliates. All rights reserved.

27a28,31

import java.io.IOException;
import java.io.InvalidObjectException;
import java.io.ObjectInputStream;
import java.io.ObjectOutputStream;

83a88

   this.longUpdateCounts = (updateCounts == null) ? null : 
copyUpdateCount(updateCounts);

192c197
< this((cause == null ? null : cause.toString()), null, 0, null, cause);
---

 this((cause == null ? null : cause.toString()), null, 0, (int[])null, 
cause);

295a301

 this.longUpdateCounts = (updateCounts == null) ? null : 
copyUpdateCount(updateCounts);

331c337,401
<
---

/**
* Constructs a BatchUpdateException object initialized with
* a given reason, SQLState, 
vendorCode
* cause and updateCounts.
* 
* This constructor should be used when the returned update count may exceed
* {@link Integer.MAX_VALUE}.
* 
* @param reason a description of the error
* @param SQLState an XOPEN or SQL:2003 code identifying the exception
* @param vendorCode an exception code used by a particular
* database vendor
* @param updateCounts an array of long, with each element
*indicating the update count, Statement.SUCCESS_NO_INFO or
* Statement.EXECUTE_FAILED for each SQL command in
* the batch for JDBC drivers that continue processing
* after a command failure; an update count or
* Statement.SUCCESS_NO_INFO for each SQL command in the batch
* prior to the failure for JDBC drivers that stop processing after a command
* failure
* @param cause the underlying reason for this SQLException
* (which is saved for later retrieval by the getCause() 
method);
* may be null indicating the cause is non-existent or unknown.
* @since 1.8
*/
   public BatchUpdateException(String reason, String SQLState, int vendorCode,
   long []updateCounts,Throwable cause) {
   super(reason, SQLState, vendorCode, cause);
   this.longUpdateCounts  = (updateCounts == null) ? null : 
Arrays.copyOf(updateCounts, updateCounts.length);
   this.updateCounts = (longUpdateCounts == null) ? null : 
copyUpdateCount(longUpdateCounts);
   }
 /**
* Retrieves the update count for each update statement in the batch
* update that executed successfully before this exception occurred.
* A driver that implements batch updates may or may not continue to
* process the remaining commands in a batch when one of the commands
* fails to execute properly. If the driver continues processing commands,
* the array returned by this method will have as many elements as
* there are commands in the batch; otherwise, it will contain an
* updat

Re: Adding field to BatchUpdateException

2012-11-26 Thread Lance Andersen - Oracle
Hi Ulf,

Thank you for the suggestion

The current spec is silent on what happens if the update count overflows for 
any executeUpdate method.  Today the driver vendors just pass in their array of 
UpdateCounts (or status) to BatchException (or overflow the return count from 
executeUpdate)  if an error occurs at some point along the way.  I really would 
prefer to not change any existing behavior here at all.  If application 
programmers care about the update count, then they will switch to new xxxLarge 
methods.

Best
Lance
On Nov 26, 2012, at 5:10 PM, Ulf Zibis wrote:

> Hi Lance,
> 
> I would throw an IllegalStateException if invoking e.g. getUpdateCounts on 
> integer overflow.
> 
> -Ulf
> 
> 
> Am 26.11.2012 20:44, schrieb Lance Andersen - Oracle:
>> Hi Joe,
>> 
>> Thank you for the sanity check.
>> 
>> I had added the following to the top of the javadoc (still playing with the 
>> wording):
>> 
>> As of Java SE 8, the method getLargeUpdateCount has been added to provide 
>> support for update counts that may be exceed Integer.MAX_VALUE and returned 
>> by the method Statement.executeLargeBatch. A JDBC driver implementation is 
>> required to throw BatchUpdateException(String reason, String SQLState, int 
>> vendorCode, long []updateCounts,Throwable cause) if an error occurs during 
>> the the execution of Statement.executeLargeBatch. If 
>> Statement.executeLargeBatch is invoked it is recommended that 
>> getLargeUpdateCounts be called instead of getUpdateCounts in order to avoid 
>> a possible overflow of the integer update count.
>> 
>> 
>> Best
>> Lance
>> 
>> On Nov 26, 2012, at 1:51 AM, Joe Darcy wrote:
>> 
>>> Hi Lance,
>>> 
>>> I don't see an obvious problem with the code, but I strongly suggest 
>>> documenting the correctness conditions regarding the updateCounts and 
>>> longUpdateCounts fields; I think that would ease reviewing the new 
>>> constructors and serialization code.
>>> 
>>> Cheers,
>>> 
>>> -Joe
>>> 
>>> On 11/24/2012 2:05 PM, Lance Andersen - Oracle wrote:
 Hi,
 
 For JDBC 4.2, I am adding methods to allow for larger update counts 
 (request from JDBC driver vendors)  and because of this I have to tweak 
 BatchUpdateException
 
 The Statement interface has the method
 
 int[] executeBatch()
 
 I am planning to add
 
 long[] executeLargeBatch().
 
 To accomodate this change, I  also need to add a new field and the method 
 getLargeUpdateCount to BatchUpdateException.
 
 I have exchanged emails on this with Alan and he indicated that the 
 changes seemed reasonable but to send a general email out to see if 
 anything was  missed from the serialization perspective.
 
 I have added JDBC Unit tests to validate that the 
 serialization/deserialization works between JDBC 4.1 and JDBC 4.2 and they 
 run without a problem.
 
 
 Best
 Lance
 
 new-host-2:sql lanceandersen$ diff BatchUpdateException.java 
 ~/NetBeansProjects/JDBC4.2/jdbc4.0/src/java/sql/
 2c2
 <  * Copyright (c) 1998, 2011, Oracle and/or its affiliates. All rights 
 reserved.
 ---
>  * Copyright (c) 1998, 2012, Oracle and/or its affiliates. All rights 
> reserved.
 27a28,31
> import java.io.IOException;
> import java.io.InvalidObjectException;
> import java.io.ObjectInputStream;
> import java.io.ObjectOutputStream;
 83a88
>   this.longUpdateCounts = (updateCounts == null) ? null : 
> copyUpdateCount(updateCounts);
 192c197
 < this((cause == null ? null : cause.toString()), null, 0, null, 
 cause);
 ---
> this((cause == null ? null : cause.toString()), null, 0, 
> (int[])null, cause);
 295a301
> this.longUpdateCounts = (updateCounts == null) ? null : 
> copyUpdateCount(updateCounts);
 331c337,401
 <
 ---
>/**
>* Constructs a BatchUpdateException object initialized 
> with
>* a given reason, SQLState, 
> vendorCode
>* cause and updateCounts.
>* 
>* This constructor should be used when the returned update count may 
> exceed
>* {@link Integer.MAX_VALUE}.
>* 
>* @param reason a description of the error
>* @param SQLState an XOPEN or SQL:2003 code identifying the exception
>* @param vendorCode an exception code used by a particular
>* database vendor
>* @param updateCounts an array of long, with each element
>*indicating the update count, Statement.SUCCESS_NO_INFO or
>* Statement.EXECUTE_FAILED for each SQL command in
>* the batch for JDBC drivers that continue processing
>* after a command failure; an update count or
>* Statement.SUCCESS_NO_INFO for each SQL command in the 
> batch
>* prior to the failure for JDBC drivers that stop processing after a 
> command
>* failure
>* @param ca

hg: jdk8/tl/jdk: 8001634: Initial set of functional interface types

2012-11-26 Thread mike . duigou
Changeset: c2e80176a697
Author:mduigou
Date:  2012-11-26 15:08 -0800
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/c2e80176a697

8001634: Initial set of functional interface types
Summary: Add the core functional interfaces used by the JSR335 libraries.
Reviewed-by: dholmes, briangoetz, darcy

! make/docs/CORE_PKGS.gmk
! make/java/java/Makefile
+ src/share/classes/java/util/function/BinaryOperator.java
+ src/share/classes/java/util/function/Block.java
+ src/share/classes/java/util/function/DoubleBinaryOperator.java
+ src/share/classes/java/util/function/DoubleBlock.java
+ src/share/classes/java/util/function/DoubleFunction.java
+ src/share/classes/java/util/function/DoubleSupplier.java
+ src/share/classes/java/util/function/DoubleUnaryOperator.java
+ src/share/classes/java/util/function/Function.java
+ src/share/classes/java/util/function/IntBinaryOperator.java
+ src/share/classes/java/util/function/IntBlock.java
+ src/share/classes/java/util/function/IntFunction.java
+ src/share/classes/java/util/function/IntSupplier.java
+ src/share/classes/java/util/function/IntUnaryOperator.java
+ src/share/classes/java/util/function/LongBinaryOperator.java
+ src/share/classes/java/util/function/LongBlock.java
+ src/share/classes/java/util/function/LongFunction.java
+ src/share/classes/java/util/function/LongSupplier.java
+ src/share/classes/java/util/function/LongUnaryOperator.java
+ src/share/classes/java/util/function/Predicate.java
+ src/share/classes/java/util/function/Supplier.java
+ src/share/classes/java/util/function/UnaryOperator.java
+ src/share/classes/java/util/function/package-info.java



Request for Review : CR#8004015 : Add interface extends and defaults for basic functional interfaces

2012-11-26 Thread Mike Duigou
Hello all;

In the original patch which added the basic lambda functional interfaces, 
CR#8001634 [1], none of the interfaces extended other interfaces. The reason 
was primarily that the javac compiler did not, at the time that 8001634 was 
proposed, support extension methods. The compiler now supports adding of method 
defaults so this patch improves the functional interfaces by filing in the 
inheritance hierarchy. 

Adding the parent interfaces and default methods allows each functional 
interface to be used in more places. It is especially important for the 
functional interfaces which support primitive types, IntSupplier, IntFunction, 
IntUnaryOperator, IntBinaryOperator, etc. We expect that eventually standard 
implementations of these interfaces will be provided for functions like max, 
min, sum, etc. By extending the reference oriented functional interfaces such 
as Function, the primitive implementations can be used with the boxed primitive 
types along with the primitive types for which they are defined.

The patch to add parent interfaces and default methods can be found here:

http://cr.openjdk.java.net/~mduigou/8004015/0/webrev/
http://cr.openjdk.java.net/~mduigou/8004015/0/specdiff/java/util/function/package-summary.html

Mike

[1] http://hg.openjdk.java.net/jdk8/tl/jdk/rev/c2e80176a697

Re: Request for Review: 5049299: (process) Use posix_spawn, not fork, on S10 to avoid swap exhaustion

2012-11-26 Thread Martin Buchholz
Rob,

Thanks for taking on this big scary change.

Our experience having run with the vfork based implementation on Linux has
been very positive.  This addresses a real need that is shared by all big
processes that desire offspring.

You might want to look through my comments from the last round of review,
back when I had more brain cells.

I agree giving people the choice to use the algorithm using a system
property is a good one.

The strategy of using posix_spawn of a small helper process seems good
(more portable than vfork or clone).  If it works well universally, you
might even consider making it the default on Linux (although I worry about
- if it works, don't break it).

clone is not known to work reliably (I tried and failed).  I would leave it
#ifdef'ed out by default.

don't put the helper program in JAVAHOME/bin because users should never
invoke it directly - it shouldn't be on anyone's PATH.

Not sure why so many dirs in HELPERLDFLAGS.  I would think that the
prochelper program would be a tiny C programs with no need to pull in any
other jdk libraries.

+String osarch = System.getProperty("os.arch");
+if (osarch.equals("sparcv9") || osarch.equals("amd64")) {
+osarch += "/";

On Solaris bi-arch I think you need only one jprochelper, not two, since a
32-bit helper can exec a 64-bit subprocess.

+#if defined(__solaris__) || defined(__APPLE__)
+#include 
+#endif

I think you want to autoconfiscate something like HAVE_POSIX_SPAWN instead.

+#ifdef _CS_PATH

I would separate out smaller less risky improvements like use of _CS_PATH
into a separate change.

Martin

On Fri, Nov 23, 2012 at 3:56 AM, Alan Bateman wrote:

> On 22/11/2012 21:27, Rob McKenna wrote:
>
>> Hi folks,
>>
>> Looking for a review for the webrev below, which also resolves:
>>
>> 7175692: (process) Process.exec should use posix_spawn [macosx]
>>
>> For additional context and a brief description it would be well worth
>> looking at the following thread started by Michael McMahon, who did the
>> brunt of the work for this fix:
>>
>> http://mail.openjdk.java.net/**pipermail/core-libs-dev/2009-**
>> May/thread.html#1644
>>
>> Basically the fix aims to swap fork for posix_spawn as the default
>> process launch mechanism on Solaris and Mac OSX in order to avoid swap
>> exhaustion issues encountered with fork()/exec(). It also offers a flag
>> (java.lang.useFork) to allow a user to revert to the old behaviour.
>>
>> I'm having trouble seeing the wood for the trees at this point so I'm
>> anticipating plenty of feedback. In particular I'd appreciate some
>> discussion on:
>>
>> - The binary launcher name & property name may need some work. A more
>> general property ("java.lang.launchMechanism") to allow a user to specify a
>> particular call was mooted too. It may be more future proof and I'm
>> completely open to that. (e.g. launchMechanism=spawn|fork|**vfork|clone
>> - we would obviously ignore inapplicable values on a per-platform basis)
>> - I'd like a more robust way of checking that someone isn't trying to use
>> jprochelper outside of the context for which it is meant.
>> - The decision around which call to use getting moved to the java level
>> and away from the native preprocessor.
>>
>> The webrev is at:
>>
>> http://cr.openjdk.java.net/~**robm/5049299/webrev.01/<
>> http://cr.openjdk.java.net/%**7Erobm/5049299/webrev.01/
>> >
>>
> It's great to get this one moving again, we should have helped Michael
> more to get this over the line on the first outing.
>
> This one will require very careful review, I don't have cycles this week,
> I hope others do. For now I think that naming the trampoline jprochelper or
> jspawnhelper is okay. To make it more robust then I'd probably prepend a
> magic number or some token. Also I'd probably fstat stdin and uses S_FIFO
> to check the mode.
>
> As the property is implementation specific then I think something like
> jdk.lang.process.useFork is a better start. It would be nice not to require
> it although I agree we have to walk carefully as this area has a tendency
> to bite back. I don't think you need to make it any more configurable than
> that.
>
> If this is successful then I think we should look at updating the hotspot
> code too as it has the same issue with VM options such as OnError.
>
> -Alan.
>
>


AnnotationParser optimization - JEP-149

2012-11-26 Thread Peter Levart

Hi all,

This might not be much of an improvement, but it is very easy to do:

--- a/src/share/classes/sun/reflect/annotation/AnnotationParser.java Thu 
Nov 15 15:40:03 2012 -0800
+++ b/src/share/classes/sun/reflect/annotation/AnnotationParser.java Tue 
Nov 27 08:39:58 2012 +0100

@@ -227,7 +227,7 @@

 Map> memberTypes = type.memberTypes();
 Map memberValues =
-new LinkedHashMap(type.memberDefaults());
+new HashMap(type.memberDefaults());

 int numMembers = buf.getShort() & 0x;
 for (int i = 0; i < numMembers; i++) {


It saves 1 OOP and 1 boolean for each Annotation instance + 2 OOPs for 
each Annotation member value.


It changes the serialization format of Annotation instances, but in a 
way that is backwards and forwards compatible.


Semantically it does not present any difference, since the only place 
that the Map is used is to Map.get(member) in the 
AnnotationInvocationHandler - it is never iterated.



Regards, Peter