Re: RFR: 8000404 java.lang.annotation.Native
On 14/11/2012 05:26, Jonathan Gibbons wrote: See http://cr.openjdk.java.net/~jjg/8000404/webrev/ A while back, we added a new annotation javax.tools.GenerateNativeHeader, to mark classes that contained constants of interest to native code, such that tools, like javac with the -h option, could generate know to generate native header files. Based on our experience in using the new annotation, we have decided to change to using a different annotation, with the same purpose: java.lang.annotation.Native, to be used to annotate the constants themselves. This review is for step 1 of a 3 step process to replace javax.tools.GenerateNativeHeader with java.lang.annotation.Native. This first step is to add the new annotation. Separately, the goal is to replacing existing uses of @GenerateNativeHeader with @Native. Then, we will remove javax.tools.GenerateNativeHeader. -- Jon I'm very happy to see @GenerateNativeHeader been replaced with @Native. The addition looks good, thumbs up from me. -Alan.
Re: Request for Review (#3) : CR#8001634 : Initial set of lambda functional interfaces
On Nov 14, 2012, at 2:19 AM, Mike Duigou mike.dui...@oracle.com wrote:
Hello all;
I apologize for the quick turnaround from the second review request [1] but
I've updated the webrev again:
http://cr.openjdk.java.net/~mduigou/8001634/4/webrev/
Looks good to me.
Blame a busy Paul Sandoz who his making significant progress on the primitive
specializations implementation. ;-)
:-) code drop to lambda repo imminent...
Paul.
This update includes:
- Block.apply renamed to Block.accept
- {Int|Long|Double}Block specializations added.
- Commented out extends CombinerT,T,T in BinaryOperator was removed for
now since Combiner is out of scope for this review.
- The {Int|Long|Double} specializations of BinaryOperator and UnaryOperator
now show commented out extends of the generic version along with commented
out default methods. This change will not be part of the commit but is meant
to show where the implementation will be going.
- The {Int|Long|Double} specializations of Supplier now extend generic
Supplier, have getAs{Int|Long|Double} as their abstract method and provide a
commented out default get() method to satisfy the need for a get
implementation.
- The {Int|Long|Double} specializations of Function now extend generic
Function, have applyAs{Int|Long|Double} as their abstract method and provide
a commented out default apply() method to satisfy the need for an apply
implementation.
Mike
[1]
http://mail.openjdk.java.net/pipermail/core-libs-dev/2012-November/012225.html
Re: Request for review: 7201156 : jar tool fails to convert file separation characters for list and extract
On 11/14/2012 05:34 AM, Jonathan Lu wrote: Hi Sean, Patch pushed @ http://hg.openjdk.java.net/jdk8/tl/jdk/rev/83765e82cacb Could you please verify? Looks fine to me. -Chris. Thanks regards Jonathan On 11/14/2012 10:23 AM, Sean Chou wrote: Thanks Alan and Chris. On Tue, Nov 13, 2012 at 7:21 PM, Chris Hegarty chris.hega...@oracle.comwrote: Sean, Looks good to me. Thanks for updating the test. -Chris. On 11/13/2012 03:17 AM, Sean Chou wrote: Hi Alan, Here is the updated webrev: http://cr.openjdk.java.net/~**zhouyx/7201156/webrev.03/http://cr.openjdk.java.net/~zhouyx/7201156/webrev.03/ . On Mon, Nov 12, 2012 at 6:00 PM, Alan Bateman alan.bate...@oracle.com** wrote: On 12/11/2012 09:08, Sean Chou wrote: Hi , I didn't realize that rt.jar would miss before. The testcase is updated to create a temp file as well as test the extract option. However, extract testing will create a directory if it passes, I just deleted it in testcase. Please take a look. webrev: http://cr.openjdk.java.net/~zhouyx/7201156/webrev.02/http://cr.openjdk.java.net/~**zhouyx/7201156/webrev.02/ http**://cr.openjdk.java.net/~**zhouyx/7201156/webrev.02/http://cr.openjdk.java.net/~zhouyx/7201156/webrev.02/ http://cr.openjdk.java.net/%7Ezhouyx/7201156/webrev.02/ht** tp://cr.openjdk.java.net/%**7Ezhouyx/7201156/webrev.02/http://cr.openjdk.java.net/%7Ezhouyx/7201156/webrev.02/ . Thanks for removing the dependency on rt.jar. A couple of comments on the updated test: - in main then it still has pathRtJar so I assume you just forgot to rename that. I forgot the name had its meaning. - it would be good to change createJarFile to use try-with-resources so that an error doesn't cause it to terminate with an open file. Changed. - testJarExact, I assume you intended to name this testJarExtract. Yes! - L114-117, the ./ is not needed. It would be okay to leave those files there if you want, jtreg will clean them up too. Thanks for the hint. I removed these lines, so the testcase looks tidy. -Alan.
Re: RFR: 7178922 : (props) re-visit how os.name is determined on Mac
On 13/11/2012 22:50, Brent Christian wrote: At present, the JDK port for OS X gets its value for os.name from a JRS function exported by the Apple Java Runtime Support framework. Historically this has either been Mac OS X, or Mac OS X Server, but there have been reports that this could change at any time, e.g. to just OS X. This would break any app that relies on this property to detect the Mac platform using something like: System.getProperty(os.name).startsWith(Mac). To ensure compatibility going forward, the os.name System property on Mac should be hard-coded to the value that is expected, Mac OS X. (FWIW, as of 10.7 Mac OS X Server is no longer a separate edition of the OS). Webrev is here: http://cr.openjdk.java.net/~bchristi/7178922/webrev.0/ Note: the setUnknownOSAndVersion() function is unused following my change, so I went ahead and removed it. Thanks, -Brent This might be a question for the MacOSX folks but is it safe to continue to depend on JavaRuntimeSupport period? I'm just wondering if we really need to use it to determine the OS version and locale? -Alan.
Re: Request for Review: CR#8001667: Comparators class and Comparator extension method
On 14 November 2012 04:09, Henry Jen henry@oracle.com wrote:
This is a change set regarding Comparator already in lambda repo, it
depends on the CR#8001634, particularly the Function SAMs.
It implements proposed extension methods on Comparator
(reverse and compose) as well as static combinator methods in
Comparators for things like turning a T - {Comparable,int,long,double}
into a ComparatorT.
This allows things like:
people.sort(Comparators.comparing(Person::getName))
ComparatorPerson byLastFirst
= Comparators.comparing(Person::getLastName)
.compose(Comparators.comparing(Person::getFirstName))
Please review and comment on the webrev[1].
Comparators declares a static serializable inner class
NaturalOrderComparator which is a singleton. Would it be worth
declaring this as an enum with a single instance? (as per Effective
Java version 2)
Comparators line 77 could do with a p
Comparators line 86: declaration about null that is covered in class
level Javadoc
Comparators line 92: who's - whose
Comparators general: I'd like to see @code around types, such as
Comparable and Map.Entry
While compose feels like the right name for Comparators, it feels
like the wrong name for the equivalent method on Comparator (given the
example below). andThen or then would be fluent alternatives.
composedWith would be a more boring alternative.
byLastName.compose(byFirstName)
byLastName.andThen(byFirstName)
byLastName.then(byFirstName)
byLastName.composedWith(byFirstName)
Stephen
Re: Request for Review (#2) : CR#8001634 : Initial set of lambda functional interfaces
On 13 November 2012 19:05, Mike Duigou mike.dui...@oracle.com wrote:
- Mapper.map becomes Function.apply
- Factory.make becomes Supplier.get
- Specializations of Supplier for int, long, double
- Reorder type variables to put result last
- Fixes many javadoc and stylistic comments.
What didn't change:
- Block was not renamed to Action or Procedure. The name Block.apply
currently conflicts with Function.apply and should be renamed. Block.accept?
Block.perform?
- Combiner will be part of the full API but it's only present in this patch
as a comment.
- No default methods.
Unless there are sustained and persuasive objections this will be committed
to the jdk8/tl workspace at the end of this week or early next week. (Hence
core-libs-dev being the primary review list)
The interface definitions say nothing about null. I'd like to see
something in there, even if it is to say null handling behaviour is
defined by the implementation.
I still find Block confusing. A code block is a block of code
delimited by { }. It takes no arguments and returns nothing.
Receiver was suggested as the interface name to match Supplier, and
that made sense to me (and is better than Sink, which is very IO).
I also find Predicate.test to be a non-optimal method name. test is
a method name I only see in test cases. Commons collections used
evaluate. is would be a possible short option. I'm sure the EG can
think of others.
Stephen
Re: RFR: 6244047: impossible to specify directories to logging FileHandler unless they exist
On 13/11/2012 21:30, Jim Gish wrote:
Here's a new webrev with my latest changes for your reviewing pleasure :-)
http://cr.openjdk.java.net/~jgish/Bug6244047-FileHandler-CheckLockLocation/
http://cr.openjdk.java.net/%7Ejgish/Bug6244047-FileHandler-CheckLockLocation/
Main changes:
- Using the file channel directly as suggested.
- Instead of checking up front for a valid directory I check the
IOException on the channel open and process it accordingly. (BTW, I
much prefer my previous proposed fix because I like to ensure
pre-conditions for an operation are met prior to it rather than
attempting the op, failing, and then recovering),
- Eliminated the stream, which really isn't needed, and just use the
file channel
Just for the purposes of my enlightenment, assuming this is what you
were after (Jason Alan), what was your issue with checking for a
valid directory up-front?
Thanks,
Jim
I get it now (I missed this on the first round), this code is using lock
files and not really using file-locking as intended.
I think the FileChannel.open usage is fine, I'm just not sure about the
exception handling. For starters, FileSystemException is a super type of
AccessDeniedException and NoSuchFileSystem so I don't think you need to
catch them specifically. Would I be correct to say that the only reason
that you would have to recover and try the next file is if the lock file
exist? In that case then maybe it just needs to be:
try {
lockFileChannel = FileChannel.open(...);
} catch (FileAlreadyExistsException ignore) {
continue;
}
-Alan.
Re: RFR: 8003322: Add instrumentation points for tracing of I/O calls
On 13/11/2012 10:16, Staffan Larsen wrote:
This is a request for review for adding tracing to I/O calls. For now, this is
an empty infrastructure intended to enable diagnosing/tracing of i/o calls. A
user of the API can register a callback for read and write operations on
sockets and files. It does not (yet) cover asynchronous i/o calls. When not
used, the implementation should add a minimum of overhead. To provide useful
information to the user, FileInputStream, FileOutputStream and RandomAccessFile
have been modified to keep track of the path they operate on (when available).
Webrev: http://cr.openjdk.java.net/~sla/8003322/webrev.00/
Thanks for the update. Do you have any updated performance data to share
(just to confirm that the updated implementation doesn't have any real
impact)?
Anyway, I took a pass over the new webrev.
I'm not sure that passing a value of 0 for errors to xxEnd is the best
approach, particularly if this is ever extended to non-blocking I/O.
Also I think there are a few inconsistencies with respect to EOF -- eg:
in FileInputStream then read() will call the hook with 0 at EOF whereas
the other read methods will call the hook with -1 at EOF. In
FileChannelImpl then some places use normalize, some not.
I guess the main question is whether the agent needs to distinguish I/O
errors from EOF and 0 bytes (the latter is assuming this may be extended
to non-blocking I/O). It may be that you need to use -2 or anything -1
to distinguish all cases.
Minor nit but there is a bit of inconsistency with the variables names,
usage of v in RandomAccessFile for example whereas FIS/FOS have
bytesRead and bytesWritten.
Thanks for adding javadoc to IoTrace. One suggestion is to include a big
warning that the hooks may be called while holding low-level locks in
the implementation and so great care must be taken, any synchronization
or interaction with other threads could easily deadlock the VM.
I skimmed over the tests (not a detailed review) and they look
reasonable. You might need to check the copyright headers as it looks
like at least one of the tests has the GPL+Classpath exception whereas
we normally use just the GPL header on tests. Also good to ensure that
there is @bug tag on the tests to link it to 8003322. In ioTraceTest.sh
I see cd ${PWD} that I didn't quite get.
Do you think these tests will be reliable when running without an images
build (meaning raw classes files on the system)? Just wondering if
expectFileRead might fail due to I/O caused by class loading.
That's all I have on the detailed review. As you mentioned there is
still have the substantive issue as to whether it's open season on
sun.misc.IoTrace*. Ignoring Unsafe (we know this needs to be
standardized or a standard alternative introduced), then nothing outside
of the JDK should be using sun.* classes directly.
-Alan
hg: jdk8/tl/jdk: 8003285: TEST_BUG: java/nio/channels/AsynchronousChannelGroup/Unbounded.java fails again [macosx]
Changeset: 0f54a98f9bc9 Author:alanb Date: 2012-11-14 12:56 + URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/0f54a98f9bc9 8003285: TEST_BUG: java/nio/channels/AsynchronousChannelGroup/Unbounded.java fails again [macosx] Reviewed-by: chegar ! test/java/nio/channels/AsynchronousChannelGroup/Unbounded.java
hg: jdk8/tl/jdk: 8000404: rename javax.tools.GenerateNativeHeader to java.lang.annotation.Native
Changeset: 369709a13823 Author:jjg Date: 2012-11-14 07:08 -0800 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/369709a13823 8000404: rename javax.tools.GenerateNativeHeader to java.lang.annotation.Native Reviewed-by: alanb + src/share/classes/java/lang/annotation/Native.java
Re: hg: jdk8/tl/jdk: 6924259: Remove offset and count fields from java.lang.String
Changing String.substring() from O(1) to O(n) is a big deal; we may say it breaks compatibility. Any code that intends to work across JDK versions before and after 7u6 cannot use the method, since its behavior is so different in different versions. Any deployment that upgrades JDK to 7u6 and later needs to review all its usages of substring(). That's a ton of work. A quick workaround might be to refactor all substring() usages to some `old_substring()` method that preserves O(1). Unfortunately `old_substring()` cannot exist, so there's no quick workaround possible. The memory leak problem of the old substring() method is well-known among Java programmers, it's not really a big problem today. For the uninitiated, they might expect that substring() is leak-free; but they might also expect that substring() is O(1). There's no apparent reason why favoring one is better than favoring the other. In the old implementation, there's a workaround to achieve leak-free, by `new String(String)`. In the new implementation, there is no workaround to achieve O(1), unless the developer uses something other than String. It is basically impossible to change String to another type if it's part of a method signature. With all these problems, please reconsider this change and see if you should roll it back. Thanks. Zhong Yu
Re: hg: jdk8/tl/jdk: 6924259: Remove offset and count fields from java.lang.String
The new implementation also introduces a new form of memory leak. Previously N substrings take O(N) space. Now it takes O(N*m) space where m is the average length of substrings. Some applications may be double penalized by the new implementation - both CPU and memory go up. On Wed, Nov 14, 2012 at 9:14 AM, Zhong Yu zhong.j...@gmail.com wrote: Changing String.substring() from O(1) to O(n) is a big deal; we may say it breaks compatibility. Any code that intends to work across JDK versions before and after 7u6 cannot use the method, since its behavior is so different in different versions. Any deployment that upgrades JDK to 7u6 and later needs to review all its usages of substring(). That's a ton of work. A quick workaround might be to refactor all substring() usages to some `old_substring()` method that preserves O(1). Unfortunately `old_substring()` cannot exist, so there's no quick workaround possible. The memory leak problem of the old substring() method is well-known among Java programmers, it's not really a big problem today. For the uninitiated, they might expect that substring() is leak-free; but they might also expect that substring() is O(1). There's no apparent reason why favoring one is better than favoring the other. In the old implementation, there's a workaround to achieve leak-free, by `new String(String)`. In the new implementation, there is no workaround to achieve O(1), unless the developer uses something other than String. It is basically impossible to change String to another type if it's part of a method signature. With all these problems, please reconsider this change and see if you should roll it back. Thanks. Zhong Yu
Re: Request for Review (#3) : CR#8001634 : Initial set of lambda functional interfaces
On 14/11/2012 01:19, Mike Duigou wrote:
Hello all;
I apologize for the quick turnaround from the second review request [1] but
I've updated the webrev again:
http://cr.openjdk.java.net/~mduigou/8001634/4/webrev/
Blame a busy Paul Sandoz who his making significant progress on the primitive
specializations implementation. ;-)
This update includes:
- Block.apply renamed to Block.accept
- {Int|Long|Double}Block specializations added.
- Commented out extends CombinerT,T,T in BinaryOperator was removed for now
since Combiner is out of scope for this review.
- The {Int|Long|Double} specializations of BinaryOperator and UnaryOperator now
show commented out extends of the generic version along with commented out
default methods. This change will not be part of the commit but is meant to
show where the implementation will be going.
- The {Int|Long|Double} specializations of Supplier now extend generic
Supplier, have getAs{Int|Long|Double} as their abstract method and provide a
commented out default get() method to satisfy the need for a get implementation.
- The {Int|Long|Double} specializations of Function now extend generic
Function, have applyAs{Int|Long|Double} as their abstract method and provide a
commented out default apply() method to satisfy the need for an apply
implementation.
Mike
[1]
http://mail.openjdk.java.net/pipermail/core-libs-dev/2012-November/012225.html
Just a few incy wincy comments on the latest webrev:
- Don't forget functions-function in make/java/java/Makefile.
- The @return in Predicate - it might read a bit better if there were a
comma before otherwise.
- In the package description it reads All functional interface
implementations are expected to but this doesn't flow well into the
bullet point. It may be better to re-word this sentence to something
like Implementators of functional interfaces should keep in mind:.
-Alan.
Re: hg: jdk8/tl/jdk: 6924259: Remove offset and count fields from java.lang.String
Hi Zhong Yu, I agree with you that changing the implementation of something like String.substring which is widely used is something that is always a little hairy. The memory leak you mention is one side of the problem, the other is that we want the VM to do memory collocation of String (i.e. allocate the array of char and the String as one object to avoid the double indirection). For that the creation of the char array and the creation of the String must be done in the constructor of String. So this change is a way to look to the bright future instead of looking to the dark past :) Now, I don't know why this change was backported to a jdk update, but it's more a question to the jdk7 update mailing list. Rémi On 11/14/2012 04:14 PM, Zhong Yu wrote: Changing String.substring() from O(1) to O(n) is a big deal; we may say it breaks compatibility. Any code that intends to work across JDK versions before and after 7u6 cannot use the method, since its behavior is so different in different versions. Any deployment that upgrades JDK to 7u6 and later needs to review all its usages of substring(). That's a ton of work. A quick workaround might be to refactor all substring() usages to some `old_substring()` method that preserves O(1). Unfortunately `old_substring()` cannot exist, so there's no quick workaround possible. The memory leak problem of the old substring() method is well-known among Java programmers, it's not really a big problem today. For the uninitiated, they might expect that substring() is leak-free; but they might also expect that substring() is O(1). There's no apparent reason why favoring one is better than favoring the other. In the old implementation, there's a workaround to achieve leak-free, by `new String(String)`. In the new implementation, there is no workaround to achieve O(1), unless the developer uses something other than String. It is basically impossible to change String to another type if it's part of a method signature. With all these problems, please reconsider this change and see if you should roll it back. Thanks. Zhong Yu
Re: hg: jdk8/tl/jdk: 6924259: Remove offset and count fields from java.lang.String
On 14/11/2012 16:06, Remi Forax wrote: Now, I don't know why this change was backported to a jdk update, but it's more a question to the jdk7 update mailing list. It was to offset the addition of the hash32 field. -Alan.
Re: Request for Review (#2) : CR#8001634 : Initial set of lambda functional interfaces
The issue is primarily when one class wants to implement more than one functional interface. If the names collide then the class will only be able to implement one of the interfaces. Mike On Nov 14 2012, at 07:12 , Craig P. Motlin wrote: What's the issue with both methods being named apply? On Tue, Nov 13, 2012 at 2:05 PM, Mike Duigou mike.dui...@oracle.com wrote: The name Block.apply currently conflicts with Function.apply and should be renamed.
hg: jdk8/tl/jdk: 7088952: Add size in bytes constant BYTES to primitive type wrapper types
Changeset: e24123de581c Author:mduigou Date: 2012-11-13 20:02 -0800 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/e24123de581c 7088952: Add size in bytes constant BYTES to primitive type wrapper types Summary: Adds a constant BYTES to each of the primitive wrapper classes (Byte, Character, Double, Float, Integer, Long, Short) with the calculation Primitive.SIZE / Byte.SIZE already made. Reviewed-by: dholmes ! src/share/classes/java/lang/Byte.java ! src/share/classes/java/lang/Character.java ! src/share/classes/java/lang/Double.java ! src/share/classes/java/lang/Float.java ! src/share/classes/java/lang/Integer.java ! src/share/classes/java/lang/Long.java ! src/share/classes/java/lang/Short.java
Re: RFR: 8000404 java.lang.annotation.Native
Looks good to me. -kto On Nov 13, 2012, at 9:26 PM, Jonathan Gibbons wrote: See http://cr.openjdk.java.net/~jjg/8000404/webrev/ A while back, we added a new annotation javax.tools.GenerateNativeHeader, to mark classes that contained constants of interest to native code, such that tools, like javac with the -h option, could generate know to generate native header files. Based on our experience in using the new annotation, we have decided to change to using a different annotation, with the same purpose: java.lang.annotation.Native, to be used to annotate the constants themselves. This review is for step 1 of a 3 step process to replace javax.tools.GenerateNativeHeader with java.lang.annotation.Native. This first step is to add the new annotation. Separately, the goal is to replacing existing uses of @GenerateNativeHeader with @Native. Then, we will remove javax.tools.GenerateNativeHeader. -- Jon
Re: RFR: 8003322: Add instrumentation points for tracing of I/O calls
Thanks for the detailed review, Alan. Comments inline.
On 14 nov 2012, at 13:50, Alan Bateman alan.bate...@oracle.com wrote:
On 13/11/2012 10:16, Staffan Larsen wrote:
This is a request for review for adding tracing to I/O calls. For now, this
is an empty infrastructure intended to enable diagnosing/tracing of i/o
calls. A user of the API can register a callback for read and write
operations on sockets and files. It does not (yet) cover asynchronous i/o
calls. When not used, the implementation should add a minimum of overhead.
To provide useful information to the user, FileInputStream, FileOutputStream
and RandomAccessFile have been modified to keep track of the path they
operate on (when available).
Webrev: http://cr.openjdk.java.net/~sla/8003322/webrev.00/
Thanks for the update. Do you have any updated performance data to share
(just to confirm that the updated implementation doesn't have any real
impact)?
While I haven't been able to measure an impact myself, I want to confirm this
with runs from the performance team. I'll get back as soon as I have something
to share.
Anyway, I took a pass over the new webrev.
I'm not sure that passing a value of 0 for errors to xxEnd is the best
approach, particularly if this is ever extended to non-blocking I/O. Also I
think there are a few inconsistencies with respect to EOF -- eg: in
FileInputStream then read() will call the hook with 0 at EOF whereas the
other read methods will call the hook with -1 at EOF. In FileChannelImpl
then some places use normalize, some not.
Thanks for catching these inconsistencies, I have fixed them.
I guess the main question is whether the agent needs to distinguish I/O
errors from EOF and 0 bytes (the latter is assuming this may be extended to
non-blocking I/O). It may be that you need to use -2 or anything -1 to
distinguish all cases.
This one is hard. As you say, it would be great to differentiate between 0
bytes, EOF and Exceptions. The first two are quite easy as I could make -1 mean
EOF. Exceptions are harder since I don't really know if there was an exception
from where the xxEnd() method is called now (typically a finally clause).
Adding a catch clause and calling xxEnd() from there would solve it, but make
the code more complicated. Hard to tell if the extra code complexity is worth
it.
Minor nit but there is a bit of inconsistency with the variables names, usage
of v in RandomAccessFile for example whereas FIS/FOS have bytesRead and
bytesWritten.
I change v to bytesRead or bytesWritten as appropriate.
Thanks for adding javadoc to IoTrace. One suggestion is to include a big
warning that the hooks may be called while holding low-level locks in the
implementation and so great care must be taken, any synchronization or
interaction with other threads could easily deadlock the VM.
I have added this.
I skimmed over the tests (not a detailed review) and they look reasonable.
You might need to check the copyright headers as it looks like at least one
of the tests has the GPL+Classpath exception whereas we normally use just the
GPL header on tests.
Fixed.
Also good to ensure that there is @bug tag on the tests to link it to 8003322.
Added.
In ioTraceTest.sh I see cd ${PWD} that I didn't quite get.
I do a few cd to different places to compile and create the jar, I then
wanted to go back to the original directory to execute the test.
Do you think these tests will be reliable when running without an images
build (meaning raw classes files on the system)? Just wondering if
expectFileRead might fail due to I/O caused by class loading.
I have been running them without an image build with no problem, but I see what
you mean. If this turns out to be a problem, then some classes may have to be
pre-loaded (such as FileInputStream, FileOutputStream, FileChannel*,
ByteBuffer).
That's all I have on the detailed review.
Thanks!
/Staffan
As you mentioned there is still have the substantive issue as to whether it's
open season on sun.misc.IoTrace*. Ignoring Unsafe (we know this needs to be
standardized or a standard alternative introduced), then nothing outside of
the JDK should be using sun.* classes directly.
-Alan
Review Request: 8001533: Java launcher must launch JavaFX applications
Bug: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=8001533 Webrev: http://cr.openjdk.java.net/~ddehaven/8001533/webrev.0/ This change adds support in the Java launcher to launch JavaFX applications directly (without the aid of launcher code injected by javafxpackager). This is accomplished by first checking if the JavaFX-Application-Class field is present in the jar manifest and if so (and a valid class) then that class is launched, otherwise Main-Class is used. Additionally, since JavaFX applications may not necessarily have a main method, if no main method is found then the main class (however specified or discovered) is checked to determine if it (eventually) extends javafx.application.Application and if so is launched via a helper class. -DrD-
Request for Review : 7175464 : entrySetView field is never updated in NavigableSubMap
Hello all; A small but useful performance fix for sub-maps of TreeMap: http://cr.openjdk.java.net/~mduigou/7175464/0/webrev/ The entrySetView was not being cached. There is no unit test because either implementation is permissible under the specification. The fix only has the effect of improving performance and reducing duplicate objects. Mike
hg: jdk8/tl/langtools: 8003412: javac needs to understand java.lang.annotation.Native
Changeset: f14c693a0e48 Author:jjg Date: 2012-11-14 10:07 -0800 URL: http://hg.openjdk.java.net/jdk8/tl/langtools/rev/f14c693a0e48 8003412: javac needs to understand java.lang.annotation.Native Reviewed-by: mcimadamore ! src/share/classes/com/sun/tools/javac/code/Symtab.java ! src/share/classes/com/sun/tools/javac/jvm/JNIWriter.java ! test/tools/javac/nativeHeaders/NativeHeaderTest.java ! test/tools/javac/nativeHeaders/javahComparison/CompareTest.java + test/tools/javac/nativeHeaders/javahComparison/TestClass4.java + test/tools/javac/nativeHeaders/javahComparison/TestClass5.java
Re: RFR: 7178922 : (props) re-visit how os.name is determined on Mac
As to the default locale detection, we need to call JavaRuntimeSupport. MacOSX's POSIX calls do not return user's preferred language/format settings. Naoto On 11/14/12 1:59 AM, Alan Bateman wrote: On 13/11/2012 22:50, Brent Christian wrote: At present, the JDK port for OS X gets its value for os.name from a JRS function exported by the Apple Java Runtime Support framework. Historically this has either been Mac OS X, or Mac OS X Server, but there have been reports that this could change at any time, e.g. to just OS X. This would break any app that relies on this property to detect the Mac platform using something like: System.getProperty(os.name).startsWith(Mac). To ensure compatibility going forward, the os.name System property on Mac should be hard-coded to the value that is expected, Mac OS X. (FWIW, as of 10.7 Mac OS X Server is no longer a separate edition of the OS). Webrev is here: http://cr.openjdk.java.net/~bchristi/7178922/webrev.0/ Note: the setUnknownOSAndVersion() function is unused following my change, so I went ahead and removed it. Thanks, -Brent This might be a question for the MacOSX folks but is it safe to continue to depend on JavaRuntimeSupport period? I'm just wondering if we really need to use it to determine the OS version and locale? -Alan.
Re: Request for Review (#2) : CR#8001634 : Initial set of lambda functional interfaces
Or when one functional interface wants to extend another, such as IntBlock extends BlockInteger On 11/14/2012 12:12 PM, Mike Duigou wrote: The issue is primarily when one class wants to implement more than one functional interface. If the names collide then the class will only be able to implement one of the interfaces. Mike On Nov 14 2012, at 07:12 , Craig P. Motlin wrote: What's the issue with both methods being named apply? On Tue, Nov 13, 2012 at 2:05 PM, Mike Duigou mike.dui...@oracle.com wrote: The name Block.apply currently conflicts with Function.apply and should be renamed.
Re: RFR: 7178922 : (props) re-visit how os.name is determined on Mac
Why not just use CFLocale and call CFLocaleCopyCurrent? https://developer.apple.com/library/mac/#documentation/CoreFoundation/Reference/CFLocaleRef/Reference/reference.html#//apple_ref/c/func/CFLocaleCopyCurrent -DrD- As to the default locale detection, we need to call JavaRuntimeSupport. MacOSX's POSIX calls do not return user's preferred language/format settings. Naoto On 11/14/12 1:59 AM, Alan Bateman wrote: On 13/11/2012 22:50, Brent Christian wrote: At present, the JDK port for OS X gets its value for os.name from a JRS function exported by the Apple Java Runtime Support framework. Historically this has either been Mac OS X, or Mac OS X Server, but there have been reports that this could change at any time, e.g. to just OS X. This would break any app that relies on this property to detect the Mac platform using something like: System.getProperty(os.name).startsWith(Mac). To ensure compatibility going forward, the os.name System property on Mac should be hard-coded to the value that is expected, Mac OS X. (FWIW, as of 10.7 Mac OS X Server is no longer a separate edition of the OS). Webrev is here: http://cr.openjdk.java.net/~bchristi/7178922/webrev.0/ Note: the setUnknownOSAndVersion() function is unused following my change, so I went ahead and removed it. Thanks, -Brent This might be a question for the MacOSX folks but is it safe to continue to depend on JavaRuntimeSupport period? I'm just wondering if we really need to use it to determine the OS version and locale? -Alan.
Request for Review : 6553074 : String{Buffer, Builder}.indexOf(Str, int) contains unnecessary allocation
Hello all; This patch causes the indexOf and lastIndexOf implementation in AbstractStringBuilder to directly compare the character arrays rather than making a copy of the substring before comparing. http://cr.openjdk.java.net/~mduigou/6553074/0/webrev/
Re: RFR: 7178922 : (props) re-visit how os.name is determined on Mac
On Nov 14, 2012, at 1:59 AM, Alan Bateman alan.bate...@oracle.com wrote: On 13/11/2012 22:50, Brent Christian wrote: At present, the JDK port for OS X gets its value for os.name from a JRS function exported by the Apple Java Runtime Support framework. Historically this has either been Mac OS X, or Mac OS X Server, but there have been reports that this could change at any time, e.g. to just OS X. This would break any app that relies on this property to detect the Mac platform using something like: System.getProperty(os.name).startsWith(Mac). To ensure compatibility going forward, the os.name System property on Mac should be hard-coded to the value that is expected, Mac OS X. (FWIW, as of 10.7 Mac OS X Server is no longer a separate edition of the OS). Webrev is here: http://cr.openjdk.java.net/~bchristi/7178922/webrev.0/ Note: the setUnknownOSAndVersion() function is unused following my change, so I went ahead and removed it. Thanks, -Brent This might be a question for the MacOSX folks but is it safe to continue to depend on JavaRuntimeSupport period? I'm just wondering if we really need to use it to determine the OS version and locale? JavaRuntimeSupport.framework was explicitly created to make API for OpenJDK and 3rd party JVMs to do everything that the Apple Java SE 6 did using private SPI. To prove that it worked, we re-implemented Java SE 6 on top of it. It's purpose is to expose a stable API for functionality that is generally inappropriate for Cocoa applications, but is necessary for the Java to cooperate with the OS X graphical environment. We currently have no plans to expand JavaRuntimeSupport, and no plans to deprecate it. Regards, Mike Swingler Apple Inc.
Re: RFR: 7178922 : (props) re-visit how os.name is determined on Mac
We do use CFLocale for the default format locale detection, which is used for formatting Date/Time/Number etc. Users can specify different language from it for the UI language, such as menu/button/etc, which can (I think) only be retrieved with that JRS function. Naoto On 11/14/12 10:21 AM, David DeHaven wrote: Why not just use CFLocale and call CFLocaleCopyCurrent? https://developer.apple.com/library/mac/#documentation/CoreFoundation/Reference/CFLocaleRef/Reference/reference.html#//apple_ref/c/func/CFLocaleCopyCurrent -DrD- As to the default locale detection, we need to call JavaRuntimeSupport. MacOSX's POSIX calls do not return user's preferred language/format settings. Naoto On 11/14/12 1:59 AM, Alan Bateman wrote: On 13/11/2012 22:50, Brent Christian wrote: At present, the JDK port for OS X gets its value for os.name from a JRS function exported by the Apple Java Runtime Support framework. Historically this has either been Mac OS X, or Mac OS X Server, but there have been reports that this could change at any time, e.g. to just OS X. This would break any app that relies on this property to detect the Mac platform using something like: System.getProperty(os.name).startsWith(Mac). To ensure compatibility going forward, the os.name System property on Mac should be hard-coded to the value that is expected, Mac OS X. (FWIW, as of 10.7 Mac OS X Server is no longer a separate edition of the OS). Webrev is here: http://cr.openjdk.java.net/~bchristi/7178922/webrev.0/ Note: the setUnknownOSAndVersion() function is unused following my change, so I went ahead and removed it. Thanks, -Brent This might be a question for the MacOSX folks but is it safe to continue to depend on JavaRuntimeSupport period? I'm just wondering if we really need to use it to determine the OS version and locale? -Alan.
Re: RFR: 6244047: impossible to specify directories to logging FileHandler unless they exist
Check out the latest, please --
http://cr.openjdk.java.net/~jgish/Bug6244047-FileHandler-CheckLockLocation/
http://cr.openjdk.java.net/%7Ejgish/Bug6244047-FileHandler-CheckLockLocation/
-- If it's ok, please push it or let me know who to have do it?
Thanks,
Jim
BTW I was expecting that NotDirectoryException would be thrown. However,
sun.nio.fs.UnixException does not translate an error code 20
(UnixConstants.ENOTDIR) to NotDirectoryException, even though it could.
Perhaps we should fix that, unless you see a reason not to. I'll check
the history, bug reports, etc. and bring it up on nio-dev unless you
know off the top of your head why we're not checking for ENOTDIR error code.
On 11/14/2012 06:08 AM, Alan Bateman wrote:
On 13/11/2012 21:30, Jim Gish wrote:
Here's a new webrev with my latest changes for your reviewing
pleasure :-)
http://cr.openjdk.java.net/~jgish/Bug6244047-FileHandler-CheckLockLocation/
http://cr.openjdk.java.net/%7Ejgish/Bug6244047-FileHandler-CheckLockLocation/
Main changes:
- Using the file channel directly as suggested.
- Instead of checking up front for a valid directory I check the
IOException on the channel open and process it accordingly. (BTW, I
much prefer my previous proposed fix because I like to ensure
pre-conditions for an operation are met prior to it rather than
attempting the op, failing, and then recovering),
- Eliminated the stream, which really isn't needed, and just use the
file channel
Just for the purposes of my enlightenment, assuming this is what you
were after (Jason Alan), what was your issue with checking for a
valid directory up-front?
Thanks,
Jim
I get it now (I missed this on the first round), this code is using
lock files and not really using file-locking as intended.
I think the FileChannel.open usage is fine, I'm just not sure about
the exception handling. For starters, FileSystemException is a super
type of AccessDeniedException and NoSuchFileSystem so I don't think
you need to catch them specifically. Would I be correct to say that
the only reason that you would have to recover and try the next file
is if the lock file exist? In that case then maybe it just needs to be:
try {
lockFileChannel = FileChannel.open(...);
} catch (FileAlreadyExistsException ignore) {
continue;
}
-Alan.
--
Jim Gish | Consulting Member of Technical Staff | +1.781.442.0304
Oracle Java Platform Group | Core Libraries Team
35 Network Drive
Burlington, MA 01803
jim.g...@oracle.com
RFR: 8003380 - Compiler warnings in logging test code
Please review http://cr.openjdk.java.net/~jgish/Bug8003380-logging-test-warnings/ http://cr.openjdk.java.net/%7Ejgish/Bug8003380-logging-test-warnings/ These are simple changes to eliminate compiler warnings from java.util.logging test code. Thanks, Jim -- Jim Gish | Consulting Member of Technical Staff | +1.781.442.0304 Oracle Java Platform Group | Core Libraries Team 35 Network Drive Burlington, MA 01803 jim.g...@oracle.com
Re: Request for Review : 6553074 : String{Buffer, Builder}.indexOf(Str, int) contains unnecessary allocation
Mike, In String.java, with the new methods you're adding, should we make those String target parameters a CharSequence instead? Thanks, Jim On 11/14/2012 01:27 PM, Mike Duigou wrote: Hello all; This patch causes the indexOf and lastIndexOf implementation in AbstractStringBuilder to directly compare the character arrays rather than making a copy of the substring before comparing. http://cr.openjdk.java.net/~mduigou/6553074/0/webrev/ -- Jim Gish | Consulting Member of Technical Staff | +1.781.442.0304 Oracle Java Platform Group | Core Libraries Team 35 Network Drive Burlington, MA 01803 jim.g...@oracle.com
Re: RFR: 7178922 : (props) re-visit how os.name is determined on Mac
Thanks, Sergey. It's good that we standardized on the recommended usage within the JDK in order to stay ahead of a possible change to the value of ProductName in /System/Library/CoreServices/SystemVersion.plist But we can expect that Java application developers use the same variety of OS platform checks. Which is why we should maintain the current value (versus risking apps breaking in the event of a change in ProductName used by OS X), especially considering that it works fine with the recommended osName.contains(OS X). As is suggested in the bug report, if the Mac's OS changes so radically that we should be reporting a new os.name (Mac OS XI, or who knows what), we will almost certainly need to update the JDK to run on it, at which time we can also change the value of os.name. Thanks, -Brent On 11/13/12 5:05 PM, Sergey Bylokhov wrote: So many efforts was done to unify this style across the jdk http://monaco.sfbay.sun.com/detail.jsf?cr=7147461 http://monaco.sfbay.sun.com/detail.jsf?cr=7130404 changesets http://hg.openjdk.java.net/jdk8/awt/jdk/rev/77b35c5c4b95 http://hg.openjdk.java.net/hsx/hotspot-rt/hotspot/rev/970cbbba54b0 http://closedjdk.us.oracle.com/hsx/hotspot-rt/hotspot/test/closed/rev/40505e5a55e8 Note that official documentation from apple suggest: contains(OS X) https://developer.apple.com/library/mac/#technotes/tn2002/tn2110.html 14.11.2012 2:50, Brent Christian wrote: At present, the JDK port for OS X gets its value for os.name from a JRS function exported by the Apple Java Runtime Support framework. Historically this has either been Mac OS X, or Mac OS X Server, but there have been reports that this could change at any time, e.g. to just OS X. This would break any app that relies on this property to detect the Mac platform using something like: System.getProperty(os.name).startsWith(Mac). To ensure compatibility going forward, the os.name System property on Mac should be hard-coded to the value that is expected, Mac OS X. (FWIW, as of 10.7 Mac OS X Server is no longer a separate edition of the OS). Webrev is here: http://cr.openjdk.java.net/~bchristi/7178922/webrev.0/ Note: the setUnknownOSAndVersion() function is unused following my change, so I went ahead and removed it. Thanks, -Brent
Re: RFR: 8003380 - Compiler warnings in logging test code
looks Ok. On Nov 14, 2012, at 4:15 PM, Jim Gish wrote: Please review http://cr.openjdk.java.net/~jgish/Bug8003380-logging-test-warnings/ http://cr.openjdk.java.net/%7Ejgish/Bug8003380-logging-test-warnings/ These are simple changes to eliminate compiler warnings from java.util.logging test code. Thanks, Jim -- Jim Gish | Consulting Member of Technical Staff | +1.781.442.0304 Oracle Java Platform Group | Core Libraries Team 35 Network Drive Burlington, MA 01803 jim.g...@oracle.com Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com
Re: RFR: 6244047: impossible to specify directories to logging FileHandler unless they exist
On 14/11/2012 20:56, Jim Gish wrote: Check out the latest, please -- http://cr.openjdk.java.net/~jgish/Bug6244047-FileHandler-CheckLockLocation/ http://cr.openjdk.java.net/%7Ejgish/Bug6244047-FileHandler-CheckLockLocation/ -- If it's ok, please push it or let me know who to have do it? I think it's okay except that you don't need to catch IOException, simply catching FileAlreadyExistsException exception should do it. If you agree then update the patch and I can push it for you. : BTW I was expecting that NotDirectoryException would be thrown. However, sun.nio.fs.UnixException does not translate an error code 20 (UnixConstants.ENOTDIR) to NotDirectoryException, even though it could. Perhaps we should fix that, unless you see a reason not to. I'll check the history, bug reports, etc. and bring it up on nio-dev unless you know off the top of your head why we're not checking for ENOTDIR error code. NotDirectoryException is for the case where you attempt do something specific to a directory but the file isn't a directory. There is special handing in newDirectoryStream for this. -Alan.
Re: RFR: 6244047: impossible to specify directories to logging FileHandler unless they exist
On 11/14/2012 04:38 PM, Alan Bateman wrote: On 14/11/2012 20:56, Jim Gish wrote: Check out the latest, please -- http://cr.openjdk.java.net/~jgish/Bug6244047-FileHandler-CheckLockLocation/ http://cr.openjdk.java.net/%7Ejgish/Bug6244047-FileHandler-CheckLockLocation/ -- If it's ok, please push it or let me know who to have do it? I think it's okay except that you don't need to catch IOException, simply catching FileAlreadyExistsException exception should do it. If you agree then update the patch and I can push it for you. But of course. Sorry I missed the obvious. Just peeling the onion when I could have taken a bite :-) Fixed, and updated. : BTW I was expecting that NotDirectoryException would be thrown. However, sun.nio.fs.UnixException does not translate an error code 20 (UnixConstants.ENOTDIR) to NotDirectoryException, even though it could. Perhaps we should fix that, unless you see a reason not to. I'll check the history, bug reports, etc. and bring it up on nio-dev unless you know off the top of your head why we're not checking for ENOTDIR error code. NotDirectoryException is for the case where you attempt do something specific to a directory but the file isn't a directory. There is special handing in newDirectoryStream for this. Exactly -- that's my point. This is one of those cases. You're trying to create a new file in a directory, but the file you specified isn't a directory - it's a plain file. The error code coming back is ENOTDIR and so what you get is the more general FileSystemException with Not a directory as the error message, when in fact, we could throw NotDirectoryException if we'd simply check for errno of ENOTDIR in the first place along with the other checks being done in UnixException translateToIOException(File,String). -Alan. -- Jim Gish | Consulting Member of Technical Staff | +1.781.442.0304 Oracle Java Platform Group | Core Libraries Team 35 Network Drive Burlington, MA 01803 jim.g...@oracle.com
Re: RFR: 7178922 : (props) re-visit how os.name is determined on Mac
On 14/11/2012 18:26, Mike Swingler wrote: : JavaRuntimeSupport.framework was explicitly created to make API for OpenJDK and 3rd party JVMs to do everything that the Apple Java SE 6 did using private SPI. To prove that it worked, we re-implemented Java SE 6 on top of it. It's purpose is to expose a stable API for functionality that is generally inappropriate for Cocoa applications, but is necessary for the Java to cooperate with the OS X graphical environment. We currently have no plans to expand JavaRuntimeSupport, and no plans to deprecate it. Regards, Mike Swingler Apple Inc. Thank you for the clear statement, this is exactly what we needed as the status of JavaRuntimeSupport.framework was not clear (at least not to me). -Alan
Re: Request for Review (#2) : CR#8001634 : Initial set of lambda functional interfaces
Implementing more than one of the functional interfaces sounds like a bad idea; making it difficult or impossible to do is a *good *thing. Use apply everywhere to prevent things like Shimmer implements FloorWax, DessertTopping. Does anyone have any compelling example of why you actually might want to implement multiple functional interfaces that isn't satisfied by using asFunction, asPredicate, asProcedure, etc. methods? --tim On Wed, Nov 14, 2012 at 12:12 PM, Mike Duigou mike.dui...@oracle.comwrote: The issue is primarily when one class wants to implement more than one functional interface. If the names collide then the class will only be able to implement one of the interfaces. Mike On Nov 14 2012, at 07:12 , Craig P. Motlin wrote: What's the issue with both methods being named apply? On Tue, Nov 13, 2012 at 2:05 PM, Mike Duigou mike.dui...@oracle.comwrote: The name Block.apply currently conflicts with Function.apply and should be renamed.
hg: jdk8/tl/langtools: 8003306: Compiler crash: calculation of inner class access modifier
Changeset: e6b1abdc11ca Author:rfield Date: 2012-11-13 08:06 -0800 URL: http://hg.openjdk.java.net/jdk8/tl/langtools/rev/e6b1abdc11ca 8003306: Compiler crash: calculation of inner class access modifier Summary: Fix binary sense lost in transition to hasTag Reviewed-by: mcimadamore ! src/share/classes/com/sun/tools/javac/comp/LambdaToMethod.java + test/tools/javac/lambda/InnerConstructor.java
code review request: Test case for JDK-7198904 TreeMap.clone issue
Hi! This is a review request to add only the test case for the following OracleJDK issue: [ 7198904 : (alt-rt) TreeMap.clone is broken ] http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7198904 The issue (root cause) is not in OpenJDK (i.e. the problem was OracleJDK specific), but the test case is valid for both so it should go into OpenJDK so we can prevent a similar issue from ever happening in both releases moving forward. webrev: [ Code Review for jdk ] http://cr.openjdk.java.net/~dbuck/7198904/webrev.00/ The OracleJDK fix (closed source) is ready and has already passed code review. I intend to push both the OracleJDK fix and this test case into their respective repositories at the same time once this review is done. Regards, -Buck
What happened to System/Process.getPid() ?
I'm sorry if I missed this, but I can't seem to find any information about what happened to the RFE provide process ID [1]. This umbrella bug report/RFE is marked as 'fixed', but I can't see that the getPid part is included in the current build of JDK8 (build 62). The process destroy/getPid RFE was previously listed under JDK8 features, but now it's gone.[2] And I can't find any information about getPid() being dropped from JDK8 either. In his review request for the System.killProcess part of this RFE, Rob McKenna wrote: As per the bug report the toString/pid work has been left to be completed separately.[3] What happened to System/Process.getPid() ? Is anyone actively working on adding this to JDK8 ? [1]. http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=4244896 [2]. http://openjdk.java.net/projects/jdk8/features [3]. http://mail.openjdk.java.net/pipermail/core-libs-dev/2012-April/009816.html Thanks Thomas
Re: Request for Review (#2) : CR#8001634 : Initial set of lambda functional interfaces
What's the issue with both methods being named apply? On Tue, Nov 13, 2012 at 2:05 PM, Mike Duigou mike.dui...@oracle.com wrote: The name Block.apply currently conflicts with Function.apply and should be renamed.
hg: jdk8/tl/langtools: 8000694: Add generation of lambda implementation code: invokedynamic call, lambda method, adaptor methods
Changeset: a65971893c50 Author:rfield Date: 2012-10-29 10:39 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/langtools/rev/a65971893c50 8000694: Add generation of lambda implementation code: invokedynamic call, lambda method, adaptor methods Summary: Add lambda implementation code with calling/supporting code elsewhere in the compiler Reviewed-by: mcimadamore, jjg ! src/share/classes/com/sun/tools/javac/code/Symtab.java + src/share/classes/com/sun/tools/javac/comp/LambdaToMethod.java ! src/share/classes/com/sun/tools/javac/comp/TransTypes.java ! src/share/classes/com/sun/tools/javac/main/JavaCompiler.java ! src/share/classes/com/sun/tools/javac/util/Names.java
Re: Request for Review (#3) : CR#8001634 : Initial set of lambda functional interfaces
Great rate of progress on this - well done.
The package Javadoc still needs some work.
Sent from my iPad
On 14/11/2012, at 1:19 AM, Mike Duigou mike.dui...@oracle.com wrote:
Hello all;
I apologize for the quick turnaround from the second review request [1] but
I've updated the webrev again:
http://cr.openjdk.java.net/~mduigou/8001634/4/webrev/
Blame a busy Paul Sandoz who his making significant progress on the primitive
specializations implementation. ;-)
This update includes:
- Block.apply renamed to Block.accept
- {Int|Long|Double}Block specializations added.
- Commented out extends CombinerT,T,T in BinaryOperator was removed for
now since Combiner is out of scope for this review.
- The {Int|Long|Double} specializations of BinaryOperator and UnaryOperator
now show commented out extends of the generic version along with commented
out default methods. This change will not be part of the commit but is meant
to show where the implementation will be going.
- The {Int|Long|Double} specializations of Supplier now extend generic
Supplier, have getAs{Int|Long|Double} as their abstract method and provide a
commented out default get() method to satisfy the need for a get
implementation.
- The {Int|Long|Double} specializations of Function now extend generic
Function, have applyAs{Int|Long|Double} as their abstract method and provide
a commented out default apply() method to satisfy the need for an apply
implementation.
Mike
[1]
http://mail.openjdk.java.net/pipermail/core-libs-dev/2012-November/012225.html
Re: hg: jdk8/tl/jdk: 6924259: Remove offset and count fields from java.lang.String
On 06/03/2012 11:35 PM, Mike Duigou wrote: [I trimmed the distribution list] On Jun 3 2012, at 13:44 , Peter Levart wrote: On Thursday, May 31, 2012 03:22:35 AM mike.duigou at oracle.com wrote: Changeset: 2c773daa825d Author:mduigou Date: 2012-05-17 10:06 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/2c773daa825d 6924259: Remove offset and count fields from java.lang.String Summary: Removes the use of shared character array buffers by String along with the two fields needed to support the use of shared buffers. Wow, that's quite a change. Indeed. It was a long time in development. It is a change which is expected to be overall beneficial though and in the general case a positive win. Wow! If the previous behavior of substring() was once a bug, by now it has become a well known feature. People know about it, and people depend on it. This change is a big surprise. Changing O(1) to O(n) is a breach of contract. It'll break lots of old code; and meanwhile lots of new code are still being written based on the old assumption. After people learned about the new behavior, they need to comb through and rewrite their code. The worst part is the same code performs very differently on different versions of JDK. What's a programmer supposed to do if his code targets JDK6 and above? If the cost of strings are no longer certain, what else can we believe in? Is there any chance in hell to roll it back? Maybe add a new method for the new behavior? Zhong Yu
Re: Review Request: CR#8001634 : Initial set of lambda functional interfaces
General Comment
=
For the collections framework there is a description of the whole framework
(http://docs.oracle.com/javase/7/docs/technotes/guides/collections/index.html);
why not do the same for the lambda framework and put a reference to the
description in all the interfaces' and classes' Javadoc. This description would
be a good place to talk about concurrency and side effects and to show
suggested usage idioms.
Overall Structure
=
I will preface this comment saying that you might be considering doing this
suggestion; but it isn't included at this stage because you are previewing a
subset that excludes default methods etc.
With the Collections Framework there are root interfaces, Collection and Map,
and progressively there are more specific interfaces. This structure is very
handy because it allows collections to be treated abstractly. Likewise it would
be handy if the functions had Combiner at the top of the hierarchy. In
particular:
public interface CombinerL,R,V {
V operate(L left, R right);
}
public interface BinaryOperatorT extends CombinerT,T,T {
T operate(T left, T right);
}
public interface UnaryOperatorT extends CombinerT,Void,T {
default T operate(T left, Void notUsed) { return operate(left); }
T operate(T operand);
}
public interface ZerothOperatorT extends CombinerVoid,Void,T {
default T operate(Void notUsed1, Void notUsed2) { return operate(); }
T operate();
}
public interface IntBinaryOperator extends BinaryOperatorInteger {
default Integer operate(Integer left, Integer right) { return
intOperate(left, right); }
int intOperate(int left, int right);
}
Similarly long and double versions.
public interface IntUnaryOperator extends UnaryOperatorInteger {
default Integer operate(Integer operand) { return intOperate(operand); }
int intOperate(int operand);
}
Similarly long and double versions.
public interface MapperT, R extends CombinerT, Void, R {
default R operate(T t, Void notUsed) { return map(T t); }
R map(T t);
}
public interface IntMapperT extends MapperT, Integer {
default Integer map(T t) { return intMap(t); }
int intMap(T t);
}
Similarly long and double versions.
public interface PredicateT extends MapperT, Boolean {
default Boolean map(T t) { return test(t); }
boolean test(T t);
}
public interface BlockT extends MapperT, Void {
default Void map(T t) { apply(t); return null; }
void apply(T t);
}
public interface FactoryT extend ZerothOperatorT {
default T operate() { return make(); }
T make();
}
Float Variation
===
There are int, long, and double specialisations; float specialisations would
also be useful (particularly for graphics). Also, some day maybe the JVM will
use the graphics co-processor (which are much faster for float than double).
Typo
The last section of package-info All functional interface implementations are
expected to: seems to be a typo. Presumably this file was only partially
completed.
Keep up the good work,
-- Howard.
Sent from my iPad
On 01/11/2012, at 7:16 AM, Mike Duigou mike.dui...@oracle.com wrote:
There's a large set of library changes that will be coming with Lambda. We're
getting near the end of the runway and there's lots left to do so we want to
start the process of getting some of the more stable pieces put back to the
JDK8 repositories. We've spent a some time slicing things into manageable
chunks. This is the first bunch. We'd like to time-box this review at one
week, since there are many more pieces to follow.
The first chunk is the basic set of functional interface types. While this
set is not complete, it is enough to be able to proceed on some other pieces.
This set contains no extension methods (we'll do those separately) and does
not contain all the specializations we may eventually need.
The specification is limited; most of the interesting restrictions
(side-effect-freedom, idempotency, stability) would really be imposed not by
the SAM itself by by how the SAM is used in a calculation. However, some
common doc for how to write good SAMs that we can stick in the package doc
would be helpful. Suggestions welcome.
Elements of this naming scheme include:
- Each SAM type has a unique (arity, method name) pair. This allows SAMs to
implement other SAMs without collision.
- The argument lists are structured so that specializations act on the first
argument(s), so IntMapperT is a specialization of MapperR,T, and
IntBinaryOperator is a specialization of BinaryOperatorT.
In order to get the most useful feedback out of this review, we'd like to ask
you follow the following guidelines for the review:
- We are time-boxed at one week. (until Nov. 7th)
- Please review the whole bunch in a single message if possible, rather than
in bits and pieces. It is far easier to extract useful feedback from one
complete review than from a dozen partial
Re: Cannot build jdk7u-dev
(bcc'ing core-libs-dev@) Looks like this is related to 6981400. I'm CC'ing Anton to take a look at it. -- best regards, Anthony On 11/7/2012 2:58 PM, Weijun Wang wrote: I've just synced with jdk7u-dev and now it does not build. symbol: class TimedWindowEvent location: class SunToolkit ../../../src/share/classes/sun/awt/SunToolkit.java:472: error: cannot find symbol TimedWindowEvent twe = (TimedWindowEvent)nested; ^ symbol: class TimedWindowEvent location: class SunToolkit It seems there is no class called TimedWindowEvent. I am building the jdk repo only. Thanks Max
Re: Review request:7197210: java/lang/invoke/CallSiteTest.java failing on armsflt
Redirecting the review request to core-libs-dev@openjdk.java.net mail list... Here is the webrev based on the jdk8/tl/jdk repository: http://cr.openjdk.java.net/~jiangli/7197210/webrev.02/ The '-XX:+IgnoreUnrecognizedVMOptions -XX:-VerifyDependencies' options are added to following tests to reduce workload. Timeout settings are also added to each test. The '-esa' option is added to BigArityTest and MethodHandlesTest to enable asserts. java.lang.invoke.BigArityTest java.lang.invoke.MethodHandlesTest java.lang.invoke.CallSiteTest Thanks, Jiangli On 10/30/2012 11:38 AM, Jiangli Zhou wrote: Hi Chris, Here is the updated webrev with added '-XX:+IgnoreUnrecognizedVMOptions -XX:-VerifyDependencies' options and timeout setting for the following tests: test.java.lang.invoke.RicochetTest test.java.lang.invoke.BigArityTest test.java.lang.invoke.MethodHandlesTest test.java.lang.invoke.CallSiteTest http://cr.openjdk.java.net/~jiangli/7197210/webrev.01/ Thanks, Jiangli
hg: jdk8/tl/jdk: 8000806: Implement runtime lambda metafactory
Changeset: 6302932b7380 Author:rfield Date: 2012-10-25 17:34 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/6302932b7380 8000806: Implement runtime lambda metafactory Summary: Implement lambda invokedynamic bootstrap by generating at runtime an inner class that implements the functional interface Reviewed-by: twisti + src/share/classes/java/lang/invoke/AbstractValidatingLambdaMetafactory.java + src/share/classes/java/lang/invoke/InnerClassLambdaMetafactory.java + src/share/classes/java/lang/invoke/LambdaConversionException.java + src/share/classes/java/lang/invoke/LambdaMetafactory.java + src/share/classes/java/lang/invoke/MagicLambdaImpl.java + src/share/classes/java/lang/invoke/TypeConvertingMethodAdapter.java
Re: RFR: 8003380 - Compiler warnings in logging test code
Interesting... fixing warnings in tests. A few comments. - LoggingMXBeanTest2.java ListIterator? - ListIteratorString and remove redundant cast ? - @SuppressWarnings(unused) Eclipse??? Do we have precedent for adding these suppressions?? - ClassLoaderLeakTest Why the change to use toURI().toURL() ?? -Chris On 14 Nov 2012, at 21:15, Jim Gish jim.g...@oracle.com wrote: Please review http://cr.openjdk.java.net/~jgish/Bug8003380-logging-test-warnings/ http://cr.openjdk.java.net/%7Ejgish/Bug8003380-logging-test-warnings/ These are simple changes to eliminate compiler warnings from java.util.logging test code. Thanks, Jim -- Jim Gish | Consulting Member of Technical Staff | +1.781.442.0304 Oracle Java Platform Group | Core Libraries Team 35 Network Drive Burlington, MA 01803 jim.g...@oracle.com
Re: RFR: 8003380 - Compiler warnings in logging test code
On 11/14/2012 05:44 PM, Chris Hegarty wrote: Interesting... fixing warnings in tests. A few comments. Right -- one might consider it overkill sine the warnings don't show up in normal testing, but they do show up in Eclipse. Just plain annoying. - LoggingMXBeanTest2.java ListIterator? - ListIteratorString and remove redundant cast ? ok. - @SuppressWarnings(unused)Eclipse??? Do we have precedent for adding these suppressions?? Not that I know of. - ClassLoaderLeakTest Why the change to use toURI().toURL() ?? Because directly applying .toURL() unless it is on a URI is deprecated. ...Jim -Chris On 14 Nov 2012, at 21:15, Jim Gish jim.g...@oracle.com mailto:jim.g...@oracle.com wrote: Please review http://cr.openjdk.java.net/~jgish/Bug8003380-logging-test-warnings/ http://cr.openjdk.java.net/%7Ejgish/Bug8003380-logging-test-warnings/ http://cr.openjdk.java.net/%7Ejgish/Bug8003380-logging-test-warnings/ These are simple changes to eliminate compiler warnings from java.util.logging test code. Thanks, Jim -- Jim Gish | Consulting Member of Technical Staff | +1.781.442.0304 Oracle Java Platform Group | Core Libraries Team 35 Network Drive Burlington, MA 01803 jim.g...@oracle.com mailto:jim.g...@oracle.com -- Jim Gish | Consulting Member of Technical Staff | +1.781.442.0304 Oracle Java Platform Group | Core Libraries Team 35 Network Drive Burlington, MA 01803 jim.g...@oracle.com
Re: RFR: 8003380 - Compiler warnings in logging test code
I've updated the webrev with your suggestion. Here it is: http://cr.openjdk.java.net/~jgish/Bug8003380-logging-test-warnings/ http://cr.openjdk.java.net/%7Ejgish/Bug8003380-logging-test-warnings/ Could someone please push it? Thanks, Jim On 11/14/2012 05:48 PM, Jim Gish wrote: On 11/14/2012 05:44 PM, Chris Hegarty wrote: Interesting... fixing warnings in tests. A few comments. Right -- one might consider it overkill sine the warnings don't show up in normal testing, but they do show up in Eclipse. Just plain annoying. - LoggingMXBeanTest2.java ListIterator? - ListIteratorString and remove redundant cast ? ok. - @SuppressWarnings(unused) Eclipse??? Do we have precedent for adding these suppressions?? Not that I know of. - ClassLoaderLeakTest Why the change to use toURI().toURL() ?? Because directly applying .toURL() unless it is on a URI is deprecated. ...Jim -Chris On 14 Nov 2012, at 21:15, Jim Gish jim.g...@oracle.com mailto:jim.g...@oracle.com wrote: Please review http://cr.openjdk.java.net/~jgish/Bug8003380-logging-test-warnings/ http://cr.openjdk.java.net/%7Ejgish/Bug8003380-logging-test-warnings/ http://cr.openjdk.java.net/%7Ejgish/Bug8003380-logging-test-warnings/ These are simple changes to eliminate compiler warnings from java.util.logging test code. Thanks, Jim -- Jim Gish | Consulting Member of Technical Staff | +1.781.442.0304 Oracle Java Platform Group | Core Libraries Team 35 Network Drive Burlington, MA 01803 jim.g...@oracle.com mailto:jim.g...@oracle.com -- Jim Gish | Consulting Member of Technical Staff | +1.781.442.0304 Oracle Java Platform Group | Core Libraries Team 35 Network Drive Burlington, MA 01803 jim.g...@oracle.com
Re: hg: jdk8/tl/jdk: 6924259: Remove offset and count fields from java.lang.String
Personally, I feel like the concern is a bit overstated: 1) the n in O(n) is likely actually fairly small in practice (at least in what I'd consider sane code) 2) I think a lot of people that worry about perf probably aren't using substring() anyway 3) copying char[] is optimized by jit - this is basically a memcpy()-like call, which modern machines handle well 4) the upside is strings are 8 bytes smaller 5) .NET substring() has always allocated new storage (via an optimized internal VM call) and never shared the char[] and I haven't come across any complaints or seen serious perf problems myself (granted I seldom use substring) So I don't know if this is anything to worry about in practice. Sent from my phone On Nov 14, 2012 5:26 PM, Zhong Yu zhong.j...@gmail.com wrote: On 06/03/2012 11:35 PM, Mike Duigou wrote: [I trimmed the distribution list] On Jun 3 2012, at 13:44 , Peter Levart wrote: On Thursday, May 31, 2012 03:22:35 AM mike.duigou at oracle.com wrote: Changeset: 2c773daa825d Author:mduigou Date: 2012-05-17 10:06 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/2c773daa825d 6924259: Remove offset and count fields from java.lang.String Summary: Removes the use of shared character array buffers by String along with the two fields needed to support the use of shared buffers. Wow, that's quite a change. Indeed. It was a long time in development. It is a change which is expected to be overall beneficial though and in the general case a positive win. Wow! If the previous behavior of substring() was once a bug, by now it has become a well known feature. People know about it, and people depend on it. This change is a big surprise. Changing O(1) to O(n) is a breach of contract. It'll break lots of old code; and meanwhile lots of new code are still being written based on the old assumption. After people learned about the new behavior, they need to comb through and rewrite their code. The worst part is the same code performs very differently on different versions of JDK. What's a programmer supposed to do if his code targets JDK6 and above? If the cost of strings are no longer certain, what else can we believe in? Is there any chance in hell to roll it back? Maybe add a new method for the new behavior? Zhong Yu
hg: jdk8/tl: 8 new changesets
Changeset: e20ffc02e437 Author:erikj Date: 2012-11-03 16:15 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/rev/e20ffc02e437 8002183: Increased max number of paths to list in ListPathsSafely to 16000. Reviewed-by: ohair ! common/makefiles/MakeBase.gmk Changeset: ed9e5635fc80 Author:erikj Date: 2012-11-03 16:28 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/rev/ed9e5635fc80 8002220: build-infra: update for mac, solaris 11 issues 8002184: Fixed exclude and includes for jarsigner in new build Reviewed-by: ohair ! common/autoconf/basics.m4 ! common/autoconf/basics_windows.m4 ! common/autoconf/compare.sh.in ! common/autoconf/generated-configure.sh ! common/autoconf/libraries.m4 ! common/bin/compare.sh ! common/bin/compare_exceptions.sh.incl ! common/makefiles/JavaCompilation.gmk Changeset: 1c8370a55b30 Author:katleman Date: 2012-11-07 15:32 -0800 URL: http://hg.openjdk.java.net/jdk8/tl/rev/1c8370a55b30 Merge Changeset: 838a64965131 Author:katleman Date: 2012-11-08 11:50 -0800 URL: http://hg.openjdk.java.net/jdk8/tl/rev/838a64965131 Added tag jdk8-b64 for changeset 1c8370a55b30 ! .hgtags Changeset: 8bbc72864a41 Author:ohrstrom Date: 2012-11-08 12:24 +0100 URL: http://hg.openjdk.java.net/jdk8/tl/rev/8bbc72864a41 8003161: small fixes to re-enable new build system Reviewed-by: dholmes, alanb, erikj ! common/makefiles/JavaCompilation.gmk Changeset: 78bb27faf889 Author:tbell Date: 2012-11-12 12:34 -0800 URL: http://hg.openjdk.java.net/jdk8/tl/rev/78bb27faf889 8002028: build-infra: need no-hotspot partial build Summary: Added configure option --with-import-hotspot=/path/to/j2sdkimage Reviewed-by: dholmes, tbell Contributed-by: erik.joels...@oracle.com ! common/autoconf/generated-configure.sh ! common/autoconf/source-dirs.m4 ! common/autoconf/spec.gmk.in ! common/makefiles/Main.gmk Changeset: f2ac4d0edaae Author:tbell Date: 2012-11-13 15:54 -0800 URL: http://hg.openjdk.java.net/jdk8/tl/rev/f2ac4d0edaae 8003274: build-infra: Makefile changes needed for sjavac Summary: changes left in build-infra that are related to sjavac Reviewed-by: ohair, tbell Contributed-by: erik.joels...@oracle.com, fredrik.ohrst...@oracle.com ! common/autoconf/spec.gmk.in ! common/makefiles/JavaCompilation.gmk ! common/makefiles/MakeHelpers.gmk Changeset: b772de306dc2 Author:katleman Date: 2012-11-14 12:28 -0800 URL: http://hg.openjdk.java.net/jdk8/tl/rev/b772de306dc2 Merge
hg: jdk8/tl/jaxp: Added tag jdk8-b64 for changeset 27ab79568c34
Changeset: 5cf3c69a93d6 Author:katleman Date: 2012-11-08 11:51 -0800 URL: http://hg.openjdk.java.net/jdk8/tl/jaxp/rev/5cf3c69a93d6 Added tag jdk8-b64 for changeset 27ab79568c34 ! .hgtags
hg: jdk8/tl/corba: Added tag jdk8-b64 for changeset 54d599a5b4aa
Changeset: 5132f7900a8f Author:katleman Date: 2012-11-08 11:50 -0800 URL: http://hg.openjdk.java.net/jdk8/tl/corba/rev/5132f7900a8f Added tag jdk8-b64 for changeset 54d599a5b4aa ! .hgtags
hg: jdk8/tl/jaxws: Added tag jdk8-b64 for changeset 5ded18a14bcc
Changeset: fbe54291c9d3 Author:katleman Date: 2012-11-08 11:51 -0800 URL: http://hg.openjdk.java.net/jdk8/tl/jaxws/rev/fbe54291c9d3 Added tag jdk8-b64 for changeset 5ded18a14bcc ! .hgtags
hg: jdk8/tl/hotspot: 18 new changesets
Changeset: 49bc14aaadcc Author:katleman Date: 2012-11-08 11:51 -0800 URL: http://hg.openjdk.java.net/jdk8/tl/hotspot/rev/49bc14aaadcc Added tag jdk8-b64 for changeset 5920f72e799c ! .hgtags Changeset: ca8168203393 Author:amurillo Date: 2012-11-02 07:44 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/hotspot/rev/ca8168203393 8002181: new hotspot build - hs25-b09 Reviewed-by: jcoomes ! make/hotspot_version Changeset: 857f3ce858dd Author:dholmes Date: 2012-11-05 19:33 -0500 URL: http://hg.openjdk.java.net/jdk8/tl/hotspot/rev/857f3ce858dd 8002034: Allow Full Debug Symbols when cross-compiling 8001756: Hotspot makefiles report missing OBJCOPY command in the wrong circumstances Reviewed-by: dcubed, dsamersoff, erikj, collins ! make/linux/makefiles/defs.make ! make/linux/makefiles/vm.make ! make/solaris/makefiles/defs.make ! make/windows/makefiles/defs.make Changeset: 3d701c802d01 Author:minqi Date: 2012-11-02 13:30 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/hotspot/rev/3d701c802d01 8000489: older builds of hsdis don't work anymore after 6879063 Summary: The old function not defined properly, need a definition for export in dll. Also changes made to let new jvm work with old hsdis. Reviewed-by: jrose, sspitsyn, kmo Contributed-by: yumin...@oracle.com ! src/share/tools/hsdis/hsdis-demo.c ! src/share/tools/hsdis/hsdis.c ! src/share/tools/hsdis/hsdis.h ! src/share/vm/compiler/disassembler.cpp ! src/share/vm/compiler/disassembler.hpp Changeset: 4735d2c84362 Author:kamg Date: 2012-10-11 12:25 -0400 URL: http://hg.openjdk.java.net/jdk8/tl/hotspot/rev/4735d2c84362 7200776: Implement default methods in interfaces Summary: Add generic type analysis and default method selection algorithms Reviewed-by: coleenp, acorn + src/share/vm/classfile/bytecodeAssembler.cpp + src/share/vm/classfile/bytecodeAssembler.hpp ! src/share/vm/classfile/classFileParser.cpp ! src/share/vm/classfile/classFileParser.hpp + src/share/vm/classfile/defaultMethods.cpp + src/share/vm/classfile/defaultMethods.hpp + src/share/vm/classfile/genericSignatures.cpp + src/share/vm/classfile/genericSignatures.hpp ! src/share/vm/classfile/systemDictionary.hpp ! src/share/vm/classfile/verifier.cpp ! src/share/vm/classfile/vmSymbols.hpp ! src/share/vm/code/dependencies.cpp ! src/share/vm/interpreter/linkResolver.cpp ! src/share/vm/oops/constMethod.cpp ! src/share/vm/oops/constMethod.hpp ! src/share/vm/oops/constantPool.cpp ! src/share/vm/oops/instanceKlass.cpp ! src/share/vm/oops/instanceKlass.hpp ! src/share/vm/oops/klassVtable.cpp ! src/share/vm/oops/klassVtable.hpp ! src/share/vm/oops/method.cpp ! src/share/vm/oops/method.hpp ! src/share/vm/runtime/globals.hpp ! src/share/vm/runtime/reflection.cpp ! src/share/vm/utilities/growableArray.hpp + src/share/vm/utilities/pair.hpp + src/share/vm/utilities/resourceHash.hpp Changeset: ec204374e626 Author:kamg Date: 2012-11-02 16:09 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/hotspot/rev/ec204374e626 Merge ! src/share/vm/classfile/vmSymbols.hpp ! src/share/vm/oops/method.cpp ! src/share/vm/runtime/globals.hpp - test/runtime/7158800/BadUtf8.java - test/runtime/7158800/InternTest.java - test/runtime/7158800/Test7158800.sh - test/runtime/7158800/badstrings.txt Changeset: 9cc901118f6b Author:kamg Date: 2012-11-02 17:18 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/hotspot/rev/9cc901118f6b Merge Changeset: 69ad7823b1ca Author:zgu Date: 2012-11-05 15:30 -0500 URL: http://hg.openjdk.java.net/jdk8/tl/hotspot/rev/69ad7823b1ca 8001591: NMT: assertion failed: assert(rec-addr() + rec-size() = cur-base()) failed: Can not overlap in memSnapshot.cpp Summary: NMT should allow overlapping committed regions as long as they belong to the same reserved region Reviewed-by: dholmes, coleenp ! src/share/vm/services/memPtr.hpp ! src/share/vm/services/memSnapshot.cpp Changeset: 8940ddc1036f Author:zgu Date: 2012-11-05 13:55 -0800 URL: http://hg.openjdk.java.net/jdk8/tl/hotspot/rev/8940ddc1036f Merge - test/runtime/7158800/BadUtf8.java - test/runtime/7158800/InternTest.java - test/runtime/7158800/Test7158800.sh - test/runtime/7158800/badstrings.txt Changeset: c284cf4781f0 Author:rbackman Date: 2012-10-04 14:55 +0200 URL: http://hg.openjdk.java.net/jdk8/tl/hotspot/rev/c284cf4781f0 7127792: Add the ability to change an existing PeriodicTask's execution interval Summary: Enables dynamic enrollment / disenrollment from the PeriodicTasks in WatcherThread. Reviewed-by: dholmes, mgronlun ! src/share/vm/runtime/mutexLocker.cpp ! src/share/vm/runtime/mutexLocker.hpp ! src/share/vm/runtime/task.cpp ! src/share/vm/runtime/task.hpp ! src/share/vm/runtime/thread.cpp ! src/share/vm/runtime/thread.hpp Changeset: 18fb7da42534 Author:coleenp Date: 2012-11-06 15:09 -0500 URL: http://hg.openjdk.java.net/jdk8/tl/hotspot/rev/18fb7da42534 8000725: NPG: method_holder()
hg: jdk8/tl/langtools: 3 new changesets
Changeset: 056d828ac1e1 Author:katleman Date: 2012-11-08 11:53 -0800 URL: http://hg.openjdk.java.net/jdk8/tl/langtools/rev/056d828ac1e1 Added tag jdk8-b64 for changeset e6ee43b3e247 ! .hgtags Changeset: 5f2faba89cac Author:lana Date: 2012-11-09 14:47 -0800 URL: http://hg.openjdk.java.net/jdk8/tl/langtools/rev/5f2faba89cac Merge Changeset: b486794d160d Author:lana Date: 2012-11-14 16:41 -0800 URL: http://hg.openjdk.java.net/jdk8/tl/langtools/rev/b486794d160d Merge
hg: jdk8/tl/jdk: 12 new changesets
Changeset: 63726e5b90da Author:erikj Date: 2012-11-03 16:27 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/63726e5b90da 8002220: build-infra: update for mac, solaris 11 issues 8002184: Fixed exclude and includes for jarsigner in new build Reviewed-by: ohair ! makefiles/CompileJavaClasses.gmk ! makefiles/CompileNativeLibraries.gmk ! makefiles/CreateJars.gmk ! makefiles/GensrcJObjC.gmk Changeset: 26dbd73fb766 Author:katleman Date: 2012-11-07 15:39 -0800 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/26dbd73fb766 Merge ! makefiles/CompileJavaClasses.gmk ! makefiles/CompileNativeLibraries.gmk ! makefiles/CreateJars.gmk Changeset: ad5c1d6b1e16 Author:katleman Date: 2012-11-08 11:52 -0800 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/ad5c1d6b1e16 Added tag jdk8-b64 for changeset 26dbd73fb766 ! .hgtags Changeset: 220d2458ce4b Author:lana Date: 2012-11-09 14:46 -0800 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/220d2458ce4b Merge Changeset: 3717bcf9d7a7 Author:dholmes Date: 2012-11-07 23:12 -0500 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/3717bcf9d7a7 8002040: Allow Full Debug Symbols when cross-compiling Reviewed-by: dcubed, erikj, tbell ! make/common/Defs-linux.gmk Changeset: 1e79fec4a01f Author:ohrstrom Date: 2012-11-08 12:25 +0100 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/1e79fec4a01f 8003161: small fixes to re-enable new build system Reviewed-by: dholmes, alanb, erikj ! makefiles/CompileNativeLibraries.gmk ! makefiles/CreateJars.gmk Changeset: 170e8ccfbc4f Author:tbell Date: 2012-11-12 10:20 -0800 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/170e8ccfbc4f 8002365: build-infra: Build-infra fails on solaris 11.1 on sparc. Summary: Add '-lc' to LDFLAGS for native libraries in CompileNativeLibraries.gmk Reviewed-by: ohair, tbell Contributed-by: erik.joels...@oracle.com ! makefiles/CompileNativeLibraries.gmk Changeset: 2fc142843a93 Author:tbell Date: 2012-11-12 10:49 -0800 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/2fc142843a93 8003177: build-infra: Compare reports diff in LocaleDataMetaInfo.class Summary: Remove spurious space in the locale lists Reviewed-by: naoto, ohair, tbell Contributed-by: erik.joels...@oracle.com ! makefiles/GensrcLocaleDataMetaInfo.gmk Changeset: e9e8a5852690 Author:tbell Date: 2012-11-12 12:35 -0800 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/e9e8a5852690 8002028: build-infra: need no-hotspot partial build Summary: Added configure option --with-import-hotspot=/path/to/j2sdkimage Reviewed-by: dholmes, tbell Contributed-by: erik.joels...@oracle.com ! makefiles/Import.gmk Changeset: 84f0439ccaab Author:tbell Date: 2012-11-13 13:46 -0800 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/84f0439ccaab 8001965: build-infra: Large compare diffs between new and old on mac Summary: The wrong icon source file was used when building closed Reviewed-by: ohair, tbell Contributed-by: erik.joels...@oracle.com ! makefiles/GensrcIcons.gmk Changeset: 130d3a54d28b Author:katleman Date: 2012-11-14 12:29 -0800 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/130d3a54d28b Merge Changeset: f4de6a38f794 Author:lana Date: 2012-11-14 16:41 -0800 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/f4de6a38f794 Merge
Re: What happened to System/Process.getPid() ?
Hi Thomas, Don't ask me why, but for some reason this mail just landed in my client now. (this happens a lot on this mailing list for some reason) getPid() is still on the todo list at the moment. Once I clear my plate a little I'll follow up on it. -Rob On 26/10/12 10:02, Thomas L wrote: I'm sorry if I missed this, but I can't seem to find any information about what happened to the RFE provide process ID [1]. This umbrella bug report/RFE is marked as 'fixed', but I can't see that the getPid part is included in the current build of JDK8 (build 62). The process destroy/getPid RFE was previously listed under JDK8 features, but now it's gone.[2] And I can't find any information about getPid() being dropped from JDK8 either. In his review request for the System.killProcess part of this RFE, Rob McKenna wrote: As per the bug report the toString/pid work has been left to be completed separately.[3] What happened to System/Process.getPid() ? Is anyone actively working on adding this to JDK8 ? [1]. http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=4244896 [2]. http://openjdk.java.net/projects/jdk8/features [3]. http://mail.openjdk.java.net/pipermail/core-libs-dev/2012-April/009816.html Thanks Thomas
hg: jdk8/tl/langtools: 7021614: extend com.sun.source API to support parsing javadoc comments
Changeset: 33abf479f202 Author:jjg Date: 2012-11-14 17:23 -0800 URL: http://hg.openjdk.java.net/jdk8/tl/langtools/rev/33abf479f202 7021614: extend com.sun.source API to support parsing javadoc comments Reviewed-by: ksrini, strarup ! make/build.xml + src/share/classes/com/sun/source/doctree/AttributeTree.java + src/share/classes/com/sun/source/doctree/AuthorTree.java + src/share/classes/com/sun/source/doctree/BlockTagTree.java + src/share/classes/com/sun/source/doctree/CommentTree.java + src/share/classes/com/sun/source/doctree/DeprecatedTree.java + src/share/classes/com/sun/source/doctree/DocCommentTree.java + src/share/classes/com/sun/source/doctree/DocRootTree.java + src/share/classes/com/sun/source/doctree/DocTree.java + src/share/classes/com/sun/source/doctree/DocTreeVisitor.java + src/share/classes/com/sun/source/doctree/EndElementTree.java + src/share/classes/com/sun/source/doctree/EntityTree.java + src/share/classes/com/sun/source/doctree/ErroneousTree.java + src/share/classes/com/sun/source/doctree/IdentifierTree.java + src/share/classes/com/sun/source/doctree/InheritDocTree.java + src/share/classes/com/sun/source/doctree/InlineTagTree.java + src/share/classes/com/sun/source/doctree/LinkTree.java + src/share/classes/com/sun/source/doctree/LiteralTree.java + src/share/classes/com/sun/source/doctree/ParamTree.java + src/share/classes/com/sun/source/doctree/ReferenceTree.java + src/share/classes/com/sun/source/doctree/ReturnTree.java + src/share/classes/com/sun/source/doctree/SeeTree.java + src/share/classes/com/sun/source/doctree/SerialDataTree.java + src/share/classes/com/sun/source/doctree/SerialFieldTree.java + src/share/classes/com/sun/source/doctree/SerialTree.java + src/share/classes/com/sun/source/doctree/SinceTree.java + src/share/classes/com/sun/source/doctree/StartElementTree.java + src/share/classes/com/sun/source/doctree/TextTree.java + src/share/classes/com/sun/source/doctree/ThrowsTree.java + src/share/classes/com/sun/source/doctree/UnknownBlockTagTree.java + src/share/classes/com/sun/source/doctree/UnknownInlineTagTree.java + src/share/classes/com/sun/source/doctree/ValueTree.java + src/share/classes/com/sun/source/doctree/VersionTree.java + src/share/classes/com/sun/source/doctree/package-info.java ! src/share/classes/com/sun/source/tree/Tree.java + src/share/classes/com/sun/source/util/DocTreeScanner.java + src/share/classes/com/sun/source/util/DocTrees.java + src/share/classes/com/sun/source/util/SimpleDocTreeVisitor.java ! src/share/classes/com/sun/source/util/Trees.java ! src/share/classes/com/sun/tools/javac/api/JavacTrees.java ! src/share/classes/com/sun/tools/javac/comp/Attr.java ! src/share/classes/com/sun/tools/javac/comp/AttrContext.java ! src/share/classes/com/sun/tools/javac/comp/Env.java ! src/share/classes/com/sun/tools/javac/comp/Resolve.java + src/share/classes/com/sun/tools/javac/parser/DocCommentParser.java ! src/share/classes/com/sun/tools/javac/parser/JavacParser.java ! src/share/classes/com/sun/tools/javac/parser/JavadocTokenizer.java + src/share/classes/com/sun/tools/javac/parser/LazyDocCommentTable.java ! src/share/classes/com/sun/tools/javac/parser/ParserFactory.java - src/share/classes/com/sun/tools/javac/parser/SimpleDocCommentTable.java ! src/share/classes/com/sun/tools/javac/resources/compiler.properties + src/share/classes/com/sun/tools/javac/tree/DCTree.java ! src/share/classes/com/sun/tools/javac/tree/DocCommentTable.java + src/share/classes/com/sun/tools/javac/tree/DocPretty.java + src/share/classes/com/sun/tools/javac/tree/DocTreeMaker.java ! src/share/classes/com/sun/tools/javadoc/DocEnv.java ! src/share/classes/com/sun/tools/javadoc/SeeTagImpl.java ! test/tools/javac/diags/CheckExamples.java + test/tools/javac/diags/DocCommentProcessor.java ! test/tools/javac/diags/Example.java ! test/tools/javac/diags/RunExamples.java ! test/tools/javac/diags/examples.not-yet.txt + test/tools/javac/diags/examples/BadEntity.java + test/tools/javac/diags/examples/BadGreaterThan.java + test/tools/javac/diags/examples/BadInlineTag.java + test/tools/javac/diags/examples/GreaterThanExpected.java + test/tools/javac/diags/examples/MalformedHTML.java + test/tools/javac/diags/examples/MissingSemicolon.java + test/tools/javac/diags/examples/NoTagName.java + test/tools/javac/diags/examples/RefBadParens.java + test/tools/javac/diags/examples/RefIdentifierExpected.java + test/tools/javac/diags/examples/RefSyntaxError.java + test/tools/javac/diags/examples/RefUnexpectedInput.java + test/tools/javac/diags/examples/UnexpectedContent.java + test/tools/javac/diags/examples/UnterminatedInlineTag.java + test/tools/javac/diags/examples/UnterminatedSignature.java + test/tools/javac/doctree/AttrTest.java + test/tools/javac/doctree/AuthorTest.java + test/tools/javac/doctree/BadTest.java + test/tools/javac/doctree/CodeTest.java + test/tools/javac/doctree/DeprecatedTest.java + test/tools/javac/doctree/DocCommentTester.java + test/tools/javac/doctree/DocRootTest.java +
Re: code review request: Test case for JDK-7198904 TreeMap.clone issue
Looks okay to me. David On 14/11/2012 11:38 PM, David Buck wrote: Hi! This is a review request to add only the test case for the following OracleJDK issue: [ 7198904 : (alt-rt) TreeMap.clone is broken ] http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7198904 The issue (root cause) is not in OpenJDK (i.e. the problem was OracleJDK specific), but the test case is valid for both so it should go into OpenJDK so we can prevent a similar issue from ever happening in both releases moving forward. webrev: [ Code Review for jdk ] http://cr.openjdk.java.net/~dbuck/7198904/webrev.00/ The OracleJDK fix (closed source) is ready and has already passed code review. I intend to push both the OracleJDK fix and this test case into their respective repositories at the same time once this review is done. Regards, -Buck
Re: hg: jdk8/tl/jdk: 6924259: Remove offset and count fields from java.lang.String
Since this change is to achieve minor performance boost, it's not fair to defend it by saying that it only incurs minor performance penalties. Java programs are infested with strings, most of which could have used a more appropriate type, but it is the insane reality. Any change to the behavior of strings should have been backed up by a much more thorough analysis. Every usage of substring() was (hopefully) the result of some conscious reasoning about space-time. Even if this change does not significantly alter an application's performance, it invalidates all the reasoning, that's the worst blow in my book. There's no problem if substring() does copying from day one, but 17 years have passed. Zhong Yu On Wed, Nov 14, 2012 at 6:58 PM, Vitaly Davidovich vita...@gmail.com wrote: Personally, I feel like the concern is a bit overstated: 1) the n in O(n) is likely actually fairly small in practice (at least in what I'd consider sane code) 2) I think a lot of people that worry about perf probably aren't using substring() anyway 3) copying char[] is optimized by jit - this is basically a memcpy()-like call, which modern machines handle well 4) the upside is strings are 8 bytes smaller 5) .NET substring() has always allocated new storage (via an optimized internal VM call) and never shared the char[] and I haven't come across any complaints or seen serious perf problems myself (granted I seldom use substring) So I don't know if this is anything to worry about in practice. Sent from my phone On Nov 14, 2012 5:26 PM, Zhong Yu zhong.j...@gmail.com wrote: On 06/03/2012 11:35 PM, Mike Duigou wrote: [I trimmed the distribution list] On Jun 3 2012, at 13:44 , Peter Levart wrote: On Thursday, May 31, 2012 03:22:35 AM mike.duigou at oracle.com wrote: Changeset: 2c773daa825d Author:mduigou Date: 2012-05-17 10:06 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/2c773daa825d 6924259: Remove offset and count fields from java.lang.String Summary: Removes the use of shared character array buffers by String along with the two fields needed to support the use of shared buffers. Wow, that's quite a change. Indeed. It was a long time in development. It is a change which is expected to be overall beneficial though and in the general case a positive win. Wow! If the previous behavior of substring() was once a bug, by now it has become a well known feature. People know about it, and people depend on it. This change is a big surprise. Changing O(1) to O(n) is a breach of contract. It'll break lots of old code; and meanwhile lots of new code are still being written based on the old assumption. After people learned about the new behavior, they need to comb through and rewrite their code. The worst part is the same code performs very differently on different versions of JDK. What's a programmer supposed to do if his code targets JDK6 and above? If the cost of strings are no longer certain, what else can we believe in? Is there any chance in hell to roll it back? Maybe add a new method for the new behavior? Zhong Yu
Re: Request for Review : 7175464 : entrySetView field is never updated in NavigableSubMap
Hi Mike, On 15/11/2012 3:49 AM, Mike Duigou wrote: Hello all; A small but useful performance fix for sub-maps of TreeMap: http://cr.openjdk.java.net/~mduigou/7175464/0/webrev/ The entrySetView was not being cached. Seems a bug that entrySetView was never being set, but given that it wasn't this may now introduce an incompatability may it not? Even if the spec permits caching for how long have we always returned a new instance? David There is no unit test because either implementation is permissible under the specification. The fix only has the effect of improving performance and reducing duplicate objects. Mike
Re: Request for review: 7201156 : jar tool fails to convert file separation characters for list and extract
That's right, thanks. On Wed, Nov 14, 2012 at 1:34 PM, Jonathan Lu luc...@linux.vnet.ibm.comwrote: Hi Sean, Patch pushed @ http://hg.openjdk.java.net/**jdk8/tl/jdk/rev/83765e82cacbhttp://hg.openjdk.java.net/jdk8/tl/jdk/rev/83765e82cacb Could you please verify? Thanks regards Jonathan On 11/14/2012 10:23 AM, Sean Chou wrote: Thanks Alan and Chris. On Tue, Nov 13, 2012 at 7:21 PM, Chris Hegarty chris.hega...@oracle.com **wrote: Sean, Looks good to me. Thanks for updating the test. -Chris. On 11/13/2012 03:17 AM, Sean Chou wrote: Hi Alan, Here is the updated webrev: http://cr.openjdk.java.net/~zhouyx/7201156/webrev.03/http://cr.openjdk.java.net/~**zhouyx/7201156/webrev.03/ http**://cr.openjdk.java.net/~**zhouyx/7201156/webrev.03/http://cr.openjdk.java.net/~zhouyx/7201156/webrev.03/ . On Mon, Nov 12, 2012 at 6:00 PM, Alan Bateman alan.bate...@oracle.com ** wrote: On 12/11/2012 09:08, Sean Chou wrote: Hi , I didn't realize that rt.jar would miss before. The testcase is updated to create a temp file as well as test the extract option. However, extract testing will create a directory if it passes, I just deleted it in testcase. Please take a look. webrev: http://cr.openjdk.java.net/~**zhouyx/7201156/webrev.02/http://cr.openjdk.java.net/~zhouyx/7201156/webrev.02/ ht**tp://cr.openjdk.java.net/~zhouyx/7201156/webrev.02/http://cr.openjdk.java.net/~**zhouyx/7201156/webrev.02/ http**://cr.openjdk.java.net/**~**zhouyx/7201156/webrev.02/http://cr.openjdk.java.net/~**zhouyx/7201156/webrev.02/ h**ttp://cr.openjdk.java.net/~**zhouyx/7201156/webrev.02/http://cr.openjdk.java.net/~zhouyx/7201156/webrev.02/ http://cr.openjdk.java.net/%**7Ezhouyx/7201156/webrev.02/**ht** tp://cr.openjdk.java.net/%7Ezhouyx/7201156/webrev.02/ht** tp://cr.openjdk.java.net/%**7Ezhouyx/7201156/webrev.02/http://cr.openjdk.java.net/%7Ezhouyx/7201156/webrev.02/ . Thanks for removing the dependency on rt.jar. A couple of comments on the updated test: - in main then it still has pathRtJar so I assume you just forgot to rename that. I forgot the name had its meaning. - it would be good to change createJarFile to use try-with-resources so that an error doesn't cause it to terminate with an open file. Changed. - testJarExact, I assume you intended to name this testJarExtract. Yes! - L114-117, the ./ is not needed. It would be okay to leave those files there if you want, jtreg will clean them up too. Thanks for the hint. I removed these lines, so the testcase looks tidy. -Alan. -- Best Regards, Sean Chou
Re: Request for Review : 7175464 : entrySetView field is never updated in NavigableSubMap
This bug appears to have been present as far back as Java 6 when NavigableSet was introduced. I could only check 6u33 but it seems unlikely to have been broken during the course of Java 6 maintenance releases. As these are views which pass through mutation to the parent object I believe there are fewer compatibility concerns than might be expected. Compatibility concern would mostly involve use of the returned entrySetView for synchronization. Previously synchronization on the returned views would have been partially ineffectual because a new view object was returned every time. Sharing the returned views would have synchronized correctly but any other access to the source object either directly or through other views would have led to problems with concurrent modification of the source treemap. In the repaired implementation where a single view object is returned synchronization on the views would now be more effective and prevent concurrent race errors. It's possible that new deadlock conditions might be introduced but this seems unlikely. Mike On Nov 14 2012, at 21:05 , David Holmes wrote: Hi Mike, On 15/11/2012 3:49 AM, Mike Duigou wrote: Hello all; A small but useful performance fix for sub-maps of TreeMap: http://cr.openjdk.java.net/~mduigou/7175464/0/webrev/ The entrySetView was not being cached. Seems a bug that entrySetView was never being set, but given that it wasn't this may now introduce an incompatability may it not? Even if the spec permits caching for how long have we always returned a new instance? David There is no unit test because either implementation is permissible under the specification. The fix only has the effect of improving performance and reducing duplicate objects. Mike
Re: Request for Review (#3) : CR#8001634 : Initial set of lambda functional interfaces
Hi Mike,
My original comment still stands regarding the wording in the Function
specializations versus all the others. Why does, for example,
IntFunction say this is the {@code int}-bearing specialization for
{@link Function}, yet IntBinaryOperator does not make a similar
statement regarding BinaryOperator?
David
On 14/11/2012 11:19 AM, Mike Duigou wrote:
Hello all;
I apologize for the quick turnaround from the second review request [1] but
I've updated the webrev again:
http://cr.openjdk.java.net/~mduigou/8001634/4/webrev/
Blame a busy Paul Sandoz who his making significant progress on the primitive
specializations implementation. ;-)
This update includes:
- Block.apply renamed to Block.accept
- {Int|Long|Double}Block specializations added.
- Commented out extends CombinerT,T,T in BinaryOperator was removed for now
since Combiner is out of scope for this review.
- The {Int|Long|Double} specializations of BinaryOperator and UnaryOperator now
show commented out extends of the generic version along with commented out
default methods. This change will not be part of the commit but is meant to
show where the implementation will be going.
- The {Int|Long|Double} specializations of Supplier now extend generic
Supplier, have getAs{Int|Long|Double} as their abstract method and provide a
commented out default get() method to satisfy the need for a get implementation.
- The {Int|Long|Double} specializations of Function now extend generic
Function, have applyAs{Int|Long|Double} as their abstract method and provide a
commented out default apply() method to satisfy the need for an apply
implementation.
Mike
[1]
http://mail.openjdk.java.net/pipermail/core-libs-dev/2012-November/012225.html
