Hi Alan,
Just one nit.
On 03/16/2016 09:30 AM, Alan Bateman wrote:
I've refreshed the webrevs here:
http://cr.openjdk.java.net/~alanb/8142968/3
In java.lang.ClassLoader:
...the package-private definePackage(String name, Module m) is OK to use
a single packages.compute(...) call performanc
> On Mar 16, 2016, at 3:18 PM, Peter Levart wrote:
>
> Hi Mandy,
>
> On 03/16/2016 10:03 PM, Mandy Chung wrote:
>> I have cleaned up the code to use toPackage instead:
>>
>> http://cr.openjdk.java.net/~mchung/jigsaw/webrevs/jigsaw-m3/webrev-03-16/index.html
>>
>>
>> Mandy
>>
>
Please se
Hi Peter,
Thanks for the review feedback.
>
> NamedPackage p = packages.get(name);
> if (p instanceof Package) {
>return (Package) p;
> }
>
> return (Package)packages.compute((n, p) -> {
> // define Package object if it is not yet defined or replace it if it is a
> NamedPackage
> return
Hi Mandy,
On 03/16/2016 10:03 PM, Mandy Chung wrote:
I have cleaned up the code to use toPackage instead:
http://cr.openjdk.java.net/~mchung/jigsaw/webrevs/jigsaw-m3/webrev-03-16/index.html
Mandy
Was your intention for public methods getDefinedPackage(name),
getPackages() and getDefinedP
On 16/03/2016 20:04, Peter Levart wrote:
:
I have another optimization...
In java.lang.reflect.Proxy, a package is added dynamically to the
module the 1st time a proxy class is defined in that module. Each time
new proxy class is defined, an array of module package names is
compiled by conc
Hi Alan,
On 03/16/2016 09:30 AM, Alan Bateman wrote:
I've refreshed the webrevs here:
http://cr.openjdk.java.net/~alanb/8142968/3
I have another optimization...
In java.lang.reflect.Proxy, a package is added dynamically to the module
the 1st time a proxy class is defined in that module. Each
> On Mar 16, 2016, at 11:50 AM, Mandy Chung wrote:
>
>> On Mar 16, 2016, at 10:30 AM, Peter Levart wrote:
>>
>> In java.lang.ClassLoader:
>>
>> ...the package-private definePackage(String name, Module m) is OK to use a
>> single packages.compute(...) call performance-wise since it is pre-scr
> On Mar 16, 2016, at 1:04 PM, Peter Levart wrote:
>
> Hi Alan,
>
> On 03/16/2016 09:30 AM, Alan Bateman wrote:
>> I've refreshed the webrevs here:
>> http://cr.openjdk.java.net/~alanb/8142968/3
>
> I have another optimization...
>
> In java.lang.reflect.Proxy, a package is added dynamically
I've refreshed the webrevs here:
http://cr.openjdk.java.net/~alanb/8142968/3
so that we have an up to date snapshot of what is currently in the
jigsaw/jake forest. The webrevs are against jdk-9+110.
Thank you to the many people that have been helping with the review and
addressing commen
Hi,
I have review the javadoc (including doclet and tests) changes. It
mostly looks good. Following are my comments on the changes
1) jdk/javadoc/internal/doclets/formats/html/ConfigurationImpl.java
- (Line 522) PackageSymbol "pd" is cast to ModuleSymbol which is incorrect.
2) jdk/javadoc
Daniel,
Thanks for the review.
> On Mar 15, 2016, at 10:48 AM, Daniel Fuchs wrote:
>
>
> 120 @Override
> 121 public int hashCode() {
> 122 int hash = 7;
> 123 hash = 67*hash + Objects.hashCode(this.filename) +
> 124 Objects.hashCode(this.path);
> 125 return hash;
> 126 }
>
> I
Mandy,
On 14 Mar 2016, at 20:37, Mandy Chung wrote:
>
>> On Mar 11, 2016, at 1:39 AM, Alan Bateman wrote:
>>
>>
>> I've refreshed the webrevs here:
>> http://cr.openjdk.java.net/~alanb/8142968/2
>
>
> I have reviewed the jmod tool and some comments:
>
> 299 private boolean printModul
Hi Alan,
I had a look at the jdeps changes and they look good.
I have a couple of minor comments:
http://cr.openjdk.java.net/~alanb/8142968/2/langtools/src/jdk.jdeps/share/classes/com/sun/tools/jdeps/Archive.java.frames.html
120 @Override
121 public int hashCode() {
122 int hash = 7;
123
Hi,
Thanks for the review. I've filed a bug to track your suggestions:
https://bugs.openjdk.java.net/browse/JDK-8151896
Thanks,
-Sundar
On 3/14/2016 6:26 PM, Michael Haupt wrote:
> Hi again,
>
> some certain list server doesn't like attachments. ;-)
> Find it at http://cr.openjdk.java.net/~mhaup
Sorry,
On 03/15/2016 02:56 PM, Peter Levart wrote:
If you also care for constant lambda, this could be optimized even
further, but for the price of more complex code:
NamedPackage p = packages.get(name);
if (p instanceof Package) {
return (Package) p;
} else if (p == null) {
Package
Hi Alan, Paul,
Just a comment on comment: wouldn't the variant with a single compute
replace a Package with itself on each repeated call? Semantically it is
the same, but there is a volatile memory store on the cache-hit involved
where in the original variant, it isn't...
On 03/11/2016 05:50
On 03/15/2016 02:22 PM, Alan Bateman wrote:
[...]
Yuri needs to look at test/java/awt/xembed/server/TesterClient.java as
that addExports usage is not right. I hope Yuri can also help on your
^ this one is historic: it is from immemorial July while the Helper
class is dated by December. I could
Thank you for the feedback Sherman.
On 14/03/16 19:50, Xueming Shen wrote:
jar.Main: comments
(1) InputstreamSupplier:
since what we really need here is the byte[], maybe just go
straightforward
to use InputStream/Files.(path)readAllBytes() ?
That is cleaner. Done.
(2) #273 don'
Thanks again for going through all the changes in this area.
I've reverted the changes to java/awt/color/ICC_Profile.java and
sun/print/ServiceDialog.java as they changes were clearly left over from
early exploration into resource encapsulation.
I've decided not to touch com/apple/laf/AquaUt
On 3/15/2016 11:24 AM, Xueming Shen wrote:
>
> On 3/14/2016 7:48 PM, Sundararajan Athijegannathan wrote:
>> Hi,
>>
>> Thanks for the review. I've filed a bug to track the cleanups
>> suggested: https://bugs.openjdk.java.net/browse/JDK-8151860
>>
>> Quick comments:
>>
>> 1) jrt fs is read-only fi
On 3/14/2016 7:48 PM, Sundararajan Athijegannathan wrote:
Hi,
Thanks for the review. I've filed a bug to track the cleanups
suggested: https://bugs.openjdk.java.net/browse/JDK-8151860
Quick comments:
1) jrt fs is read-only file system as you noted. Copying content into
jrt fs does not make s
Hi,
Thanks for the review. I've filed a bug to track the cleanups
suggested: https://bugs.openjdk.java.net/browse/JDK-8151860
Quick comments:
1) jrt fs is read-only file system as you noted. Copying content into
jrt fs does not make sense. Eventually read-only file system exception
is thrown.
B
(5) JrtFileSystemProvider.copy(src, dst, ...options)
The "dst/target" should not be cast to "AbstractJrtPath"? the jrtfs itself is
a readonly, I'm not sure if we want to support the operation of copying
a "file" output jrtfs. The copy/paste "copyToTarget" appears to be functional,
if it's not a
(1) JrtFilePath: it does not seem to help performance to use the byte[] as the
internal storage.
(2) AbstractJrtFilesystem.java
getPathMatcher:
if (syntax.equalsIgnoreCase(GLOB_SYNTAX)) {
expr = JrtUtils.toRegexPattern(input);
} else
> On Mar 11, 2016, at 1:39 AM, Alan Bateman wrote:
>
>
> I've refreshed the webrevs here:
> http://cr.openjdk.java.net/~alanb/8142968/2
I have reviewed the jmod tool and some comments:
299 private boolean printModuleDescriptor(InputStream in)
jmod -p option prints the output in differ
On 14/03/2016 19:37, Phil Race wrote:
:
http://cr.openjdk.java.net/~alanb/8142968/2/jdk/src/java.desktop/macosx/classes/com/apple/laf/AquaUtils.java.sdiff.html
368 // can we use ClassLoader.getSystemClassLoader here?
369 final ClassLoader launcherClassLoader =
ClassLoade
jar.Main: comments
(1) InputstreamSupplier:
since what we really need here is the byte[], maybe just go straightforward
to use InputStream/Files.(path)readAllBytes() ?
(2) #273 don't the "moduleInfo" used for consistency check the same one as
the used for updating at #244? can't b
http://cr.openjdk.java.net/~alanb/8142968/2/jdk/test/java/awt/patchlib/java.desktop/java/awt/Helper.java.html
30 //java.awt.Helper.class.getModule().addExports(pn, target);
Can we remove the commented out line ?
http://cr.openjdk.java.net/~alanb/8142968/2/jdk/src/java.desktop/macosx/c
These are the changes I plan on submitting for M3.
http://cr.openjdk.java.net/~jlaskey/jimagereview/webrev/index.html
I’ve filed the following bugs as follow up.
https://bugs.openjdk.java.net/browse/JDK-8151806 JImage decompress code needs
to be revised to be more effective
https://bugs.openjdk
Hi again,
some certain list server doesn't like attachments. ;-)
Find it at http://cr.openjdk.java.net/~mhaupt/jigsaw/
Best,
Michael
> Am 14.03.2016 um 13:49 schrieb Michael Haupt :
>
> Alan, all,
>
> please find a patch with suggested changes to jlink in the attachment. The
> patch also con
Alan, all,
please find a patch with suggested changes to jlink in the attachment. The
patch also contains a file named review-comments.txt, which addresses several
topics throughout jlink. I've covered most code; only the jlink.internal
package is still missing. I've been able to compile jake w
> On Mar 11, 2016, at 1:19 PM, Paul Sandoz wrote:
>
>>
>> On 11 Mar 2016, at 21:53, Mandy Chung wrote:
>>
>> I agree. It's also used from ResourceBundle.Control::newBundle. Since core
>> reflection now gets readability for free, setAccessible is no longer needed
>> [1]. So it can call Class
> On 11 Mar 2016, at 21:53, Mandy Chung wrote:
>
> Thanks for the feedback.
>
>> On Mar 11, 2016, at 8:50 AM, Paul Sandoz wrote:
>> :
>>
>> sun/util/locale/provider/ResourceBundleProviderSupport.java
>> —
>>
>>
>> 72 // java.base may not be able to read the bundleClass's
>>
> On 11 Mar 2016, at 18:53, Alan Bateman wrote:
>
> On 11/03/2016 16:50, Paul Sandoz wrote:
>> :
>> I browsed the code. Generally the feeling i get is it’s of good quality.
>> Previously some of this area was quite legacy and it is refreshing to view
>> more modern code.
> Thanks for going thr
Thanks for the feedback.
> On Mar 11, 2016, at 8:50 AM, Paul Sandoz wrote:
> :
>
> sun/util/locale/provider/ResourceBundleProviderSupport.java
> —
>
>
> 72 // java.base may not be able to read the bundleClass's
> module.
> 73 PrivilegedAction pa1 = () -> {
>
Myself (mullan), Tony (ascarpino), and Vinnie (vinnie) reviewed the
security-libs and java.naming changes and did not find any issues. So
it's good to go from our perspective.
--Sean
On 03/11/2016 04:39 AM, Alan Bateman wrote:
I've refreshed the webrevs here:
http://cr.openjdk.java.net/~
On 11/03/2016 16:50, Paul Sandoz wrote:
:
I browsed the code. Generally the feeling i get is it’s of good quality.
Previously some of this area was quite legacy and it is refreshing to view more
modern code.
Thanks for going through it. There is quite a mix here, lots of new code
but lots of a
> On 11 Mar 2016, at 10:39, Alan Bateman wrote:
>
>
> I've refreshed the webrevs here:
> http://cr.openjdk.java.net/~alanb/8142968/2
>
I browsed the code. Generally the feeling i get is it’s of good quality.
Previously some of this area was quite legacy and it is refreshing to view more
m
A non issue. I discussed using NIO buffers with Alan. That is the direction
we will go.
> On Mar 11, 2016, at 11:36 AM, Mandy Chung wrote:
>
> We rework the VM initialization such that lambdas can be used very early
> after phase 1 initialization is done and before module system initializat
We rework the VM initialization such that lambdas can be used very early after
phase 1 initialization is done and before module system initialization.
I’m not sure where jimage uses of lambdas for this case. In general, we wanted
to enable use of new language features early during startup and w
Thanks Jim.
Beware of use of Lambdas, jimage may be used early enough in the boot
cycle that lambdas
are not yet fully operative. The failure is pretty obvious but can be
trial and error to narrow it down.
It would be useful to create an jira entry for the changes that are
being delayed.
I've refreshed the webrevs here:
http://cr.openjdk.java.net/~alanb/8142968/2
so that we have a snapshot of what is currently in the jigsaw/jake
forest. The webrevs are against jdk-9+109.
As I said in the last mail, we would like to integrate this snapshot
into JDK 9 before the end of Marc
Thank you Roger. Comments inline. In general, I will hold off changes until
after merge so as not to destabilize. Note that ImageBufferCache and
Decompression are not currently used.
> On Mar 8, 2016, at 2:07 PM, Roger Riggs wrote:
>
> Hi,
>
> Review comments for the jimage code in java.ba
On 09/03/2016 08:58, Alan Bateman wrote:
On 08/03/2016 22:06, Seán Coffey wrote:
I'm hoping we can bulk up some of the new exception handling. I've
put suggested edits for the jdk repo together in a webrev. No major
edits here but it should help supportability of the code for the
future. Wil
On 08/03/2016 18:07, Roger Riggs wrote:
Hi,
Review comments for the jimage code in java.base, mostly cleanup but
perhaps a bug or two.
General: inconsistent style and not following guidelines, copyright
dates may need an update.
Use of assert for error checking is not going to catch or help
On 08/03/2016 22:06, Seán Coffey wrote:
I'm hoping we can bulk up some of the new exception handling. I've put
suggested edits for the jdk repo together in a webrev. No major edits
here but it should help supportability of the code for the future.
Will it be possible to import these in before
> On Mar 8, 2016, at 1:37 PM, Naoto Sato wrote:
>
> Hello,
>
> I reviewed ResourceBundle code and related locale data changes. Overall it
> looks good to me. Here are some minor comments:
>
> java.util.ResourceBundle.java
>
> - In the class description, there are two occurrences of example e
I'm hoping we can bulk up some of the new exception handling. I've put
suggested edits for the jdk repo together in a webrev. No major edits
here but it should help supportability of the code for the future. Will
it be possible to import these in before the bulk push ?
http://cr.openjdk.java.n
Hello,
I reviewed ResourceBundle code and related locale data changes. Overall
it looks good to me. Here are some minor comments:
java.util.ResourceBundle.java
- In the class description, there are two occurrences of example
explaing service provider type (i.e., basename+"Provider"). It seem
Hi,
Review comments for the jimage code in java.base, mostly cleanup but
perhaps a bug or two.
General: inconsistent style and not following guidelines, copyright
dates may need an update.
Use of assert for error checking is not going to catch or help isolate
errors in production builds.
di
On 08/03/2016 00:15, Mandy Chung wrote:
:
src/java.management/share/classes/sun/management/ThreadInfoCompositeData.java
197 private static final String[] threadInfoV9Attributes = {
198 DAEMON,
199 PRIORITY,
200 MODULE_NAME,
201 MODULE_VERSION,
Are
I've refreshed the webrevs here:
http://cr.openjdk.java.net/~alanb/8142968/1
so that we have a snapshot of what is currently in the jigsaw/jake
forest. The webrves are against jdk-9+108.
I plan to send mail to jdk9-dev soon proposing that we integrate a
snapshot into JDK 9 before the end o
> On Mar 3, 2016, at 6:38 AM, Alan Bateman wrote:
>
> I've pushed webrevs with the initial changes for JDK 9 here:
>http://cr.openjdk.java.net/~alanb/8142968/0/
I have reviewed changes for java.management and java.logging module.
src/java.management/share/classes/sun/management/ThreadInfoC
53 matches
Mail list logo
