Re: Review Request JDK-8201793: (ref) Reference object should not support cloning

2018-04-27 Thread mandy chung
On 4/28/18 6:08 AM, Kim Barrett wrote: On Apr 26, 2018, at 10:16 AM, mandy chung <mandy.ch...@oracle.com> wrote: The semantics of cloning a Reference object is not clearly defined. In addition, it is questionable whether it can be meaningfully supported in particular with conc

Review Request JDK-8202113: Reflection API is causing caller classes to leak

2018-04-28 Thread mandy chung
Webrev: http://cr.openjdk.java.net/~mchung/jdk11/webrevs/8202113/webrev.00/ The reflection machinery stores the caller class in each AccessibleObject such that it can skip the access check if access to a member has been verified for a given caller.  At the first time accessing the

Re: Review Request JDK-8201793: (ref) Reference object should not support cloning

2018-04-27 Thread mandy chung
subtypes to implement Cloneable.  So this change should have low compatibility risk. Mandy Paul. On Apr 26, 2018, at 7:16 AM, mandy chung <mandy.ch...@oracle.com> wrote: The semantics of cloning a Reference object is not clearly defined. In addition, it is questionable whether

Re: RFR: 8201274: Launch Single-File Source-Code Programs

2018-05-09 Thread mandy chung
On 5/4/18 2:59 PM, Jonathan Gibbons wrote: Here's an update to the previously proposed patch for JEP 330: Launch Single-File Source-Code Programs. It includes all review feedback so far. The changes are mostly minor, but with the addition of more test cases. The webrev includes a

Re: [11] RFR: 8202553: Update FXLauncherTest as part of removing JavaFX from JDK

2018-05-10 Thread mandy chung
On 5/10/18 5:59 AM, Kevin Rushforth wrote: Here is the updated webrev: http://cr.openjdk.java.net/~kcr/8202553/webrev.01/ +1 This stripped down version of FX test module looks better.  FYI. @build /* will build a module under the source directory of the test.   What you have (doing the

Re: [PATCH] JDK-8159797 Throw a right IllegalArgumentException from the bytecode generated by Method/ConstructorAccessor

2018-05-11 Thread mandy chung
Hi Jaikiran, Thanks for the contribution. With your patch, IAE thrown with empty message is less desirable even though it does not provide less information than the current message (NPE::toString).   I wonder if we could define an exception message in MethodAccessorImpl for this IAE or even

Re: Review Request JDK-8202113: Reflection API is causing caller classes to leak

2018-05-11 Thread mandy chung
On 4/30/18 10:21 AM, Alan Bateman wrote: The updated webrev looks good. A minor comment is that I assume you can remove the cast from Executable::declaredAnnotations if you leave Executable::getRoot in place. It could but leave it as is.  I found that this change breaks the hack that

Re: RFR: 8202583: Remove experimental ClassForNamePlugin

2018-05-07 Thread mandy chung
On 5/7/18 6:23 AM, Claes Redestad wrote: Hi, the --class-for-name jlink plugin was added as an experiment to test and evolve the jlink plugin infrastructure. The actual effect of enabling this plugin on standard images is very limited (only a handful of Class.forName:s are replaced) and

Re: Review Request JDK-8202113: Reflection API is causing caller classes to leak

2018-05-19 Thread mandy chung
Hi Peter, Sorry for the late reply.  I was on vacation last week and just returned. On 5/14/18 8:43 AM, Peter Levart wrote: Hi Mandy, Sorry for noticing this too late, but... If it was not necessary to keep legacy hacky behavior (to honor the patched "modifiers" field), wouldn't it be

Re: RFR: Small cleanups in java.lang.ref

2018-05-19 Thread mandy chung
On 5/18/18 7:47 AM, Martin Buchholz wrote: On the contrary, I'm very happy to see gc team actively maintaining java.lang.ref. I've reverted changes to Reference.java http://cr.openjdk.java.net/~martin/webrevs/jdk/Reference-cleanup-1/ This looks okay.

Re: [core-libs] RFR (L): 8010319: Implementation of JEP 181: Nest-Based Access Control

2018-05-21 Thread mandy chung
On 5/20/18 10:57 PM, David Holmes wrote: - I suspect the @throws SecurityException in getNestMembers was copied from getNestHost as it uses "returned class" (singular). It refers to "If any returned class ..." and "that returned class". I don't see any problematic singular uses - can you

Re: [core-libs] RFR (L): 8010319: Implementation of JEP 181: Nest-Based Access Control

2018-05-21 Thread mandy chung
On 5/21/18 9:39 AM, Paul Sandoz wrote: On May 20, 2018, at 11:32 PM, David Holmes wrote: 3984 public Class[] getNestMembers() { I still think not removing dups is a mistake as it could be a source of subtle bugs. But i doubt at this point i can persuade you or

Re: RFR: 8177276: MethodHandles.insertArguments doesn't specify IllegalArgumentException on index mismatch

2018-05-21 Thread mandy chung
On 5/16/18 5:51 PM, Vivek Theeyarath wrote: Hi All, Please review fix for https://bugs.openjdk.java.net/browse/JDK-8177276 . http://cr.openjdk.java.net/~vtheeyarath/8177276/webrev.02/ 3489  * @throws ClassCastException If an argument does not mach the corresponding bound

Re: [core-libs] RFR (L): 8010319: Implementation of JEP 181: Nest-Based Access Control

2018-05-21 Thread mandy chung
On 5/21/18 5:48 PM, David Holmes wrote: http://mail.openjdk.java.net/pipermail/valhalla-dev/2018-March/003971.html and as I responded to Alan, for getNestMembers() it doesn't say "the returned class" it says "any returned class" and "that returned class". There is no singular/plural

Re: RFR: 8177276: MethodHandles.insertArguments doesn't specify IllegalArgumentException on index mismatch

2018-05-22 Thread mandy chung
On 5/22/18 9:09 AM, Vivek Theeyarath wrote: Hi All, Thanks for the comments. I have incorporated the changes as per Nadeesh's and Paul's comments. The test runs fine with jtreg post the changes. Also, the typo errors Mandy pointed out has also been fixed. Please find the updated

Re: [core-libs] RFR (L): 8010319: Implementation of JEP 181: Nest-Based Access Control

2018-05-21 Thread mandy chung
On 5/21/18 8:41 PM, David Holmes wrote: Class.java 3988 Class[] members = getNestMembers0(); If the above fails with any LinkageError, what is the expected behavior if Class::getNestMembers() is called the second time? will it throw the same LinkageError? Based on the implementation it

Re: [core-libs] RFR (L): 8010319: Implementation of JEP 181: Nest-Based Access Control

2018-05-21 Thread mandy chung
Hi David, I reviewed: http://cr.openjdk.java.net/~dholmes/8010319-JEP181/webrev.corelibs.v1/ Looks good in general.  I have a couple other comments.  I will review the new version that includes new tests in test/jdk/java/lang/reflect when it's ready. Class.java 3988 Class[] members =

Re: [core-libs] RFR (L): 8010319: Implementation of JEP 181: Nest-Based Access Control

2018-05-22 Thread mandy chung
On 5/22/18 6:08 PM, David Holmes wrote: Will make the suggested changes for v3 (once I've processed hotspot v2 stuff). Unless you need to send a new webrev for other's comments, the test update I suggest don't need a new webrev.  You can fix it in your local repo before you push. Mandy

Re: [core-libs] RFR (L): 8010319: Implementation of JEP 181: Nest-Based Access Control

2018-05-22 Thread mandy chung
On 5/22/18 3:15 AM, David Holmes wrote: Here are the updates so far in response to all the review comments. Incremental webrev: http://cr.openjdk.java.net/~dholmes/8010319-JEP181/webrev.corelibs.v2-incr/ Full webrev: http://cr.openjdk.java.net/~dholmes/8010319-JEP181/webrev.corelibs.v2/

Re: RFR: 8177276: MethodHandles.insertArguments doesn't specify IllegalArgumentException on index mismatch

2018-05-22 Thread mandy chung
+1 Mandy On 5/22/18 6:00 PM, Vivek Theeyarath wrote: Thanks for the comments Mandy. I have updated the test accordingly. http://cr.openjdk.java.net/~vtheeyarath/8177276/webrev.04/ <http://cr.openjdk.java.net/%7Evtheeyarath/8177276/webrev.04/> Regards Vivek *From:*mandy chung

Re: RFR: 8203028: Simplify reference processing in light of JDK-8175797

2018-05-22 Thread mandy chung
Hi Kim, Thanks for doing this work and rewriting the comments.  It's great to see this simplification.  It looks good in general.  I have a couple of personal preferences if you are okay to update them. I like to keep the existing indentation of the description of different states (line

Re: [core-libs] RFR (L): 8010319: Implementation of JEP 181: Nest-Based Access Control

2018-05-22 Thread mandy chung
On 5/22/18 3:36 AM, Peter Levart wrote: In jl.Class: 3911 // returning a different class requires a security check 3912 SecurityManager sm = System.getSecurityManager(); 3913 if (sm != null) { 3914 checkPackageAccess(sm, 3915

Re: RFR: 8203028: Simplify reference processing in light of JDK-8175797

2018-05-25 Thread mandy chung
On 5/25/18 1:28 AM, Kim Barrett wrote:> I've adopted your suggestion of ASCII-art to describe the state> transitions. I've fixed a couple of errors and made some other changes to (I hope) improve readability. I've also made some naming changes for consistency and clarity, including: -

Re: [core-libs] RFR (L): 8010319: Implementation of JEP 181: Nest-Based Access Control

2018-05-25 Thread mandy chung
Looks good. Mandy On 5/24/18 10:52 PM, David Holmes wrote: Here are the further minor updates so far in response to all the review comments. Incremental corelibs webrev: http://cr.openjdk.java.net/~dholmes/8010319-JEP181/webrev.corelibs.v3-incr/ Full corelibs webrev:

Re: RFR: 81820709 - Container Awareness JEP

2018-05-24 Thread mandy chung
On 5/23/18 7:39 AM, Bob Vandette wrote: Should this be an instance method? like cpuacct.getLongValue("cpuacct.usage”); > I did it this way in order to provide a centralized place to check for missing subsystems. The getLongValue method does the checking for all subsystems 137 if

Re: RFR: 81820709 - Container Awareness JEP

2018-05-24 Thread mandy chung
On 5/24/18 12:31 PM, Bob Vandette wrote: Yes, I saw that but wasn’t sure how new text that’s added to the launcher.properties file would get localized. Is there a process for getting this done? No action should be needed. Changes to .properties files are tracked and localized version will

Re: RFR (S): 8203500: Fix broken links to Specification in "specs" directory

2018-05-21 Thread mandy chung
The fix looks fine. Mandy On 5/21/18 3:54 PM, Iris Clark wrote: Hi. Please review this small change to fix a few broken links to Specification in the “specs” directory from the JavaDoc API. The incorrect references either not include "{@docRoot}" or include the non-existent

Re: Ping!! Re: RFR: 8203357 Container Metrics

2018-06-11 Thread mandy chung
On 6/11/18 10:12 PM, David Holmes wrote: For the Java code ... methods that return arrays should return zero-length arrays when something is not available rather than null. All methods do return zero length arrays except I missed the getPerCpuUsage.  I’ll fix that one and correct the

Re: RFR: 8196993: Resolve disabled warnings for libunpack

2018-06-11 Thread mandy chung
Looks fine. Mandy On 6/11/18 10:21 PM, Srinivas Dama wrote: Gentle reminder . May I have review for the below changeset. Regards, Srinivas -Original Message- From: Srinivas Dama Sent: Friday, June 08, 2018 6:33 PM To: Core-Libs-Dev Subject: RFR: 8196993: Resolve disabled warnings

Re: [core-libs] RFR (L): 8010319: Implementation of JEP 181: Nest-Based Access Control

2018-06-11 Thread mandy chung
On 6/11/18 10:16 PM, David Holmes wrote: Here is one further minor update from the CSR discussions: http://cr.openjdk.java.net/~dholmes/8010319-JEP181/webrev.corelibs.v5-incr/src/java.base/share/classes/java/lang/Class.java.cdiff.html "This implementation" is fine, as used in many

Re: Ping!! Re: RFR: 8203357 Container Metrics

2018-06-12 Thread mandy chung
On 6/11/18 10:43 PM, David Holmes wrote: On 12/06/2018 3:31 PM, mandy chung wrote: On 6/11/18 10:12 PM, David Holmes wrote: For the Java code ... methods that return arrays should return zero-length arrays when something is not available rather than null. All methods do return zero length

Re: RFR 8204565 : (spec) Document java.{vm.}?specification.version system properties' relation to $FEATURE

2018-06-08 Thread mandy chung
On 6/8/18 12:11 PM, Brent Christian wrote: On 6/7/18 1:24 PM, mandy chung wrote: Issue: https://bugs.openjdk.java.net/browse/JDK-8204565 Webrev: http://cr.openjdk.java.net/~bchristi/8204565/webrev/ Is there an existing test validating this? Looks like there is (kind of), for libs

Re: [JDK 11] RFR 8201528: Add new test to check for package versioning information in OpenJDK

2018-06-08 Thread mandy chung
On 6/8/18 12:07 AM, Chris Yin wrote: Hi, Mandy Many thanks for your detailed review and comments, updates new webrev as below, and comment inline, thanks webrev: http://cr.openjdk.java.net/~xyin/8201528/webrev.01/ Thanks for adding the test description, that is very helpful. This is

Re: RFR: JDK-8204588: Test failures after "Launch Single-File Source-Code Programs"

2018-06-08 Thread mandy chung
On 6/8/18 3:06 PM, Jonathan Gibbons wrote: Please review two test fixes related to the source launcher feature. In one test, the fix is to use File.separator to construct "golden output" for comparison. In the other test, the failure was caused by excessively long paths to the Java

Re: RFR 8204565 : (spec) Document java.{vm.}?specification.version system properties' relation to $FEATURE

2018-06-07 Thread mandy chung
Hi Brent, On 6/7/18 12:13 PM, Brent Christian wrote: Hi, Please review this doc-only change.  From the bug report: 'With the integration of JEP 322 "Time-Based Release Versioning" into JDK 10, VERSION_FEATURE is used to set the value of the system properties "java.specification.version" [1]

Re: [core-libs] RFR (L): 8010319: Implementation of JEP 181: Nest-Based Access Control

2018-06-13 Thread mandy chung
:38 PM, mandy chung wrote: On 6/11/18 10:16 PM, David Holmes wrote: Here is one further minor update from the CSR discussions: http://cr.openjdk.java.net/~dholmes/8010319-JEP181/webrev.corelibs.v5-incr/src/java.base/share/classes/java/lang/Class.java.cdiff.html "This implementation&quo

Re: [JDK 11] RFR 8201528: Add new test to check for package versioning information in OpenJDK

2018-06-11 Thread mandy chung
On 6/10/18 10:12 PM, Chris Yin wrote: Hi, Mandy Thanks lot for your suggestions, update webrev as below and comment inline, thanks http://cr.openjdk.java.net/~xyin/8201528/webrev.02/ Looks good. Thanks for the update and this is much easier to understand the test cases. Mandy

Re: [JDK 11] RFR 8201528: Add new test to check for package versioning information in OpenJDK

2018-06-07 Thread mandy chung
Hi Chris, On 6/7/18 1:32 AM, Chris Yin wrote: Please review below new added test to check for package versioning information which customized in jar(s) manifest, many thanks bug: https://bugs.openjdk.java.net/browse/JDK-8201528 webrev: http://cr.openjdk.java.net/~xyin/8201528/webrev.00/

(XS) Review Request JDK-8204584: jdeps generates illegal dot file containing ranksep=0,600000

2018-06-07 Thread mandy chung
The dot files are generated and used as module graph in the docs build. The format of double should use no localization; otherwise an illegal ranksep attribute. Mandy diff --git a/src/jdk.jdeps/share/classes/com/sun/tools/jdeps/ModuleDotGraph.java

Review Request: 8204648: test/jdk/tools/launchers/SourceMode.java fails with long shebang line

2018-06-08 Thread mandy chung
JDK-8204588 [1] fixed the test failure caused by long paths to the Java launcher in some test execution environments, causing the shebang line to overflow the underlying system limit of 128 characters. The test needs a small tweak to the max javaCmd length to reduce from 100 to 90 since the

Re: Review Request: 8204648: test/jdk/tools/launchers/SourceMode.java fails with long shebang line

2018-06-08 Thread mandy chung
est = shortJavaCmd.toString().length() > 100; +// skipShebangTest = shortJavaCmd.toString().length() > 90; +skipShebangTest = true; } log = System.err; On 6/8/18 9:29 PM, mandy chung wrote: JDK-8204588 [1] fixed the test failure caused by long paths to the Java

Re: JDK 11 RFR of JDK-8205003: Replace selected link tags with linkplain in java.lang.Class

2018-06-13 Thread mandy chung
+1 Mandy On 6/13/18 5:51 PM, joe darcy wrote: Hello, Please review the small patch below to address     JDK-8205003: Replace selected link tags with linkplain in java.lang.Class Thanks, -Joe diff -r 0742a087710e src/java.base/share/classes/java/lang/Class.java ---

Re: RFR: 8196988 (Resolve disabled warnings for libjimage)

2018-06-12 Thread mandy chung
Have you verified the build on all platforms (release and debug)? Mandy On 6/11/18 10:23 PM, Srinivas Dama wrote: Gentle reminder. May I have review for the below changeset. Regards, Srinivas -Original Message- From: Srinivas Dama Sent: Friday, June 08, 2018 11:47 AM To:

Re: RFR 8197930: JNI exception pending in initializeEncoding of jni_util.c

2018-06-12 Thread mandy chung
Looks good. Mandy On 6/12/18 7:17 AM, Roger Riggs wrote: Please review a cleanup to not ignore errors from GetMethodID and GetFieldID while initializing the system encoding. Webrev:    http://cr.openjdk.java.net/~rriggs/webrev-jnicheck-8197930/ issue:   

Re: RFR: 8203886: Invoke LambdaMetafactory::altMetafactory exactly from the BootstrapMethodInvoker

2018-05-29 Thread mandy chung
On 5/28/18 6:00 AM, Claes Redestad wrote: Hi, similarly to JDK-8198418[1], we can avoid creation of various LambdaForms and SpeciesData types in various benchmarks by adapting calls to the altMetafactory as exactly as possible in the BootstrapMethodInvoker. Webrev:

Re: RFR: 8201274: Launch Single-File Source-Code Programs

2018-05-30 Thread mandy chung
I reviewed the delta-webrev (v3). Looks good. Thanks for fixing missing newlines in launcher.properties Mandy On 5/30/18 12:00 PM, Jonathan Gibbons wrote: Please review a minor update to the proposed implementation of JEP 330. The primary change is to disallow the use of the "shebang"

Re: RFR: 8203357 Container Metrics

2018-05-31 Thread mandy chung
Hi Bob, On 5/30/18 12:45 PM, Bob Vandette wrote:> RFE: Container Metrics https://bugs.openjdk.java.net/browse/JDK-8203357 WEBREV: http://cr.openjdk.java.net/~bobv/8203357/webrev.00 Looks fine in general. It's good to have this internal API ready for JFR and other library code to use. I

Re: RFR: 8203357 Container Metrics

2018-06-01 Thread mandy chung
On 6/1/18 8:52 AM, Bob Vandette wrote: I filed a CSR for this a few days ago. > https://bugs.openjdk.java.net/browse/JDK-8204107 Typo: s/-XshowSetting/-XshowSettings In the specification section, you can include the new lines adding to java --extra-help output (that's the spec) and drop

Re: jlink / jmods version compatibiltiy

2018-06-01 Thread mandy chung
On 6/1/18 3:24 PM, Bernd Eckenfels wrote: Hello, I am not sure what the Policy for backward/Forward compatibility for JMOD files is, but when I use JDK-9.0.4 jlink on 11ea JMODs I get a IllegalArgumentException and „error reading“ by JDK-10.0.1 with no further Details. jlink only supports

Re: RFR: JDK-8203891: Upgrade JOpt Simple to 5.0.4

2018-06-05 Thread mandy chung
On 6/5/18 12:29 AM, Jan Lahoda wrote: Yes, that would work as well, but there are already invocations like this in the test. So I opted for using "--libs=" + ... and similar, so that both variants are covered by the test. That is fine to keep it as is. The change looks good. Mandy

Re: Draft JEP proposal: JDK-8200758: Packaging Tool

2018-06-06 Thread mandy chung
On 5/30/18 5:10 PM, Kevin Rushforth wrote: I would like to propose the following Draft JEP [1] for discussion. JDK-8200758: Packaging Tool This is intended as a JDK-scope replacement for the existing javapackager tool that ships with Oracle JDK 10 (and earlier Oracle JDK releases), and

Re: RFR: JDK-8203891: Upgrade JOpt Simple to 5.0.4

2018-06-04 Thread mandy chung
Hi Jan, On 5/31/18 2:11 AM, Jan Lahoda wrote: Hi, I'd like to upgrade the JOpt Simple library we are using to version 5.0.4. Bug: https://bugs.openjdk.java.net/browse/JDK-8203891 Complete webrev: http://cr.openjdk.java.net/~jlahoda/8203891/webrev.00/complete/ Delta webrev only showing

Re: RFR: 8205438: Re-enable shebang tests in test/jdk/tools/launchers/SourceMode.java

2018-06-27 Thread mandy chung
Looks good. It's amazing to find out 3 different behavior on these 3 platforms. Mandy On 6/27/18 1:37 PM, Jonathan Gibbons wrote: Please review a test fix to re-enable shebang tests in test/jdk/tools/launchers/SourceMode.java . The test cases for invoking the source launcher via the

Re: RFR 8202292 : test/jdk/java/io/FileOutputStream/UnreferencedFOSClosesFd.java fails with "raw fd count wrong"

2018-06-22 Thread mandy chung
+1 Mandy On 6/22/18 7:49 AM, Roger Riggs wrote: Hi Brian, Mandy, Listing the open file descriptors can be a useful utility. Originally I thought it was a temporary and throw away code. To be a utility it needed to be a bit more general (sending the output to a particular stream). Thanks

Review Request: JDK-8205623: Replace use of Class::getPackage with Class::getPackageName

2018-06-25 Thread mandy chung
This patch replaces the use of Class::getPackage with Class::getPackageName in jdk.internal.reflect.ReflectionFactory, sun.util.resources.BreakIteratorResourceBundle, and javax.xml.catalog.CatalogMessages. Class::getPackageName avoids the overhead of constructing Package objects.

Re: RFR: JDK-8205158: Update the .md files for 3rd party software Unicode 10.0, ICU 60.1, and CLDR v33

2018-06-22 Thread mandy chung
-8202537 Linked mentioned issues with this one. Thanks, Rachna On 6/22/18 9:57 PM, mandy chung wrote: On 6/22/18 6:54 AM, Rachna Goel wrote: Hi, Kindly review fix to update legal files for Unicode, CLDR and ICU. Issue: https://bugs.openjdk.java.net/browse/JDK-8205158 Patch :http

Re: RFR: JDK-8205158: Update the .md files for 3rd party software Unicode 10.0, ICU 60.1, and CLDR v33

2018-06-22 Thread mandy chung
On 6/22/18 6:54 AM, Rachna Goel wrote: Hi, Kindly review fix to update legal files for Unicode, CLDR and ICU. Issue: https://bugs.openjdk.java.net/browse/JDK-8205158 Patch :http://cr.openjdk.java.net/~rgoel/JDK-8205158/webrev.02/ Looks okay. What are the issues that upgrades these

Re: [core-libs] RFR (L): 8010319: Implementation of JEP 181: Nest-Based Access Control

2018-06-19 Thread mandy chung
The javadoc update looks good to me. Mandy On 6/19/18 9:56 PM, David Holmes wrote: Some further adjustments to getNestMembers() was made. Everything updated in place. Thanks, David On 20/06/2018 9:30 AM, David Holmes wrote: Sorry another update is imminent ... stay tuned. David On

Re: RFR JDK-8066709 Make some JDK system properties read only

2018-06-26 Thread mandy chung
Looks good to me. The part that I'm unsure is whether you caught all the callers that are outside doPrivileged to StaticProperty.* will need to do its property permission check. This requires API inspection and testing that I assume you covered that. Mandy On 6/26/18 7:10 PM, Roger Riggs

Re: RRF: 8187123: (reflect) Class#getCanonicalName and Class#getSimpleName is a part of performance issue

2018-05-02 Thread mandy chung
On 4/30/18 5:49 AM, Claes Redestad wrote: Hi, please review this patch to enable caching of getCanonicalName and getSimpleName, repeated calls of which has been reported to be a performance bottleneck. The caching improves performance of these methods by up to 20x. Rather than adding new

Re: RFR: 8202419: Avoid creating Permission constants early

2018-04-30 Thread mandy chung
On 4/30/18 8:25 PM, Claes Redestad wrote: Hi, moving a couple of local permission constants to sun.security.util.SecurityConstants ensures we delay and avoid loading permission classes very early during bootstrap. Delaying load is profitable since it's happening early enough (before

Re: Review Request JDK-8202113: Reflection API is causing caller classes to leak

2018-04-30 Thread mandy chung
On 4/30/18 7:39 PM, Alan Bateman wrote: The approach looks good, seems like this one was lurking (for protected members at least) for a long time. Yes and this issue becomes more noticeable in JDK 9 as public members needs additional module access check. The 3 x getRoot methods on

Re: RRF: 8187123: (reflect) Class#getCanonicalName and Class#getSimpleName is a part of performance issue

2018-05-03 Thread mandy chung
On 5/3/18 5:07 AM, Claes Redestad wrote: newReflectionData could be updated to copy the simpleName and canonicalName from oldReflectionData.  The perf isn't a concern when the class is being redefined.  As the class name is unchanged, it might be good to copy the names from older

Re: Review Request JDK-8202113: Reflection API is causing caller classes to leak

2018-04-29 Thread mandy chung
On 4/28/18 6:52 PM, Peter Levart wrote: Hi Mandy, On 04/28/18 11:44, mandy chung wrote: Webrev: http://cr.openjdk.java.net/~mchung/jdk11/webrevs/8202113/webrev.00/ The reflection machinery stores the caller class in each AccessibleObject such that it can skip the access check if access

Re: Decreased latency performance with Stack Walker API compared to sun.misc.JavaLangAccess

2017-10-20 Thread mandy chung
Hi Rafael, Thanks for the feedback.  We did some investigation in understanding the overhead of Throwable if it used StackWalker API [1].  It did come to mind whether the StackWalker API should provide a way to walk the backtrace which we should do the investigation with JDK-8141239. The

Re: RFR 8189319 : Add a java.util.Properties constructor that takes an initial capacity

2017-10-27 Thread mandy chung
On 10/26/17 6:43 PM, Brent Christian wrote: Hi, It would be useful to have a Properties constructor that takes an argument to set the initial capacity. Such a constructor is present on many of the other Map implementations in the JDK, including Hashtable, the superclass of Properties. In

Re: [Proposal/Question] Provide mechanism to monitor thread states more efficiently

2017-10-27 Thread mandy chung
serviceability-dev is the mailing list for discussing java.lang.management APIs. Getting the stack trace is expensive. ThreadMXBean.getThreadInfos(ids) does not get the stack trace and locks information and is less expensive. A typical lightweight monitoring application does BCI and call

Re: RFR 8189319 : Add a java.util.Properties constructor that takes an initial capacity

2017-10-27 Thread mandy chung
On 10/27/17 2:37 PM, Brent Christian wrote: On 10/27/2017 11:19 AM, mandy chung wrote: It may be cleaner to initialize the map in a single place e.g. a private constructor taking Properties and initialCapacity. Yeah, that's a good idea.  See new webrev: http://cr.openjdk.java.net/~bchristi

Re: RFR: 8188055: (ref) Add Reference.refersTo predicate

2020-04-08 Thread Mandy Chung
I agree with Gil on this point.   `PhantomReference::get` always returns null.   The intent behavior of `ref.refersTo(obj)` is the same as `ref.get() == obj`.    Gil's proposed option to have `refersTo(obj)` to return true only if obj is null is a reasonable one. If `PhantomReference::get`

Re: RFR 8241749: Remove the Nashorn JavaScript Engine

2020-04-15 Thread Mandy Chung
This looks okay to me. Can you add the bug ID next to @ignore that will make it easier to find the JBS issue?   These test bugs are filed with P4 but I think they should be fixed in 15. Mandy On 4/15/20 8:56 AM, sundararajan.athijegannat...@oracle.com wrote: Please review. Nashorn script

Re: Review Request: 8238358: Implementation of JEP 371: Hidden Classes

2020-04-16 Thread Mandy Chung
Svc spec change webrev: http://cr.openjdk.java.net/~mchung/valhalla/webrevs/hidden-classes/webrev.06-svc-spec-changes/ thanks Mandy [1] https://mail.openjdk.java.net/pipermail/valhalla-dev/2020-April/007155.html Paul. On Apr 11, 2020, at 8:13 PM, Mandy Chung wrote: Please review the delta

Re: Review Request: 8238358: Implementation of JEP 371: Hidden Classes

2020-04-16 Thread Mandy Chung
On 4/16/20 11:42 AM, serguei.spit...@oracle.com wrote: Hi Mandy, I have a couple of minor comments on the Serviceability spec update.

Re: Review Request: 8238358: Implementation of JEP 371: Hidden Classes

2020-04-17 Thread Mandy Chung
On 4/17/20 3:51 PM, Chris Plummer wrote: Hi Mandy, Thanks for updating the svc specs. Some comments below: In the JDWP spec update, you changed "JNI signature" to "type signature" in one place, but left it as "JNI signature" everywhere else. Should they all be changed? JDWP signature

Re: Review Request: 8238358: Implementation of JEP 371: Hidden Classes

2020-04-06 Thread Mandy Chung
On 4/6/20 9:56 AM, serguei.spit...@oracle.com wrote: The suggested fix is: http://cr.openjdk.java.net/~sspitsyn/webrevs/2020/valhalla-jdi-regression-8242166.1/ This patch looks okay. I'll include in my local patch. On 4/6/20 11:00 AM, Chris Plummer wrote: I think that's fine but I don't

Re: Review Request: 8238358: Implementation of JEP 371: Hidden Classes

2020-04-11 Thread Mandy Chung
a proxy class 3. include Serguei's patch to fix JDK-8242166: regression in JDI ClassUnload events 4. drop the uniqueness guarantee of the suffix of the hidden class's name On 3/31/20 8:01 PM, Mandy Chung wrote: Thanks to the review feedbacks. Updated webrev: http://cr.openjdk.java.net/~mchung/v

Re: Review Request: 8238358: Implementation of JEP 371: Hidden Classes

2020-04-18 Thread Mandy Chung
On 4/18/20 12:47 AM, Chris Plummer wrote: It's a link to https://download.java.net/java/early_access/jdk15/docs/specs/jni/types.html#type-signatures. This is how the current JVM TI spec defines. Since it looks like this reference was present before your changes, I guess it's ok to leave

Review Request JDK-8240975: Extend NativeLibraries to support explicit unloading

2020-03-13 Thread Mandy Chung
Webrev: http://cr.openjdk.java.net/~mchung/jdk15/webrevs/8240975/webrev.00/ This is a follow-up task for Panama to allow explicit unloading of native library after JDK-8228336.  `NativeLibraries` associated with a class loader has the capability to auto unload native libraries when the class

Re: Review Request: 8238358: Implementation of JEP 371: Hidden Classes

2020-03-31 Thread Mandy Chung
opyright headers 9. Fix @modules in tests Most of the changes above have also been reviewed as separate patches. Thanks Mandy On 3/26/20 4:57 PM, Mandy Chung wrote: Please review the implementation of JEP 371: Hidden Classes. The main changes are in core-libs and hotspot runtime area.  Small cha

Re: Review Request: 8238358: Implementation of JEP 371: Hidden Classes

2020-04-04 Thread Mandy Chung
Hi Peter, On 4/4/20 3:58 AM, Peter Levart wrote: Here I think, you are not quite right. First I need to clarify that we are talking about the case where the method reference in above example is not converted to lambda by javac, so the proxy class needs to invoke the superclass method

Re: Review Request: 8238358: Implementation of JEP 371: Hidden Classes

2020-04-04 Thread Mandy Chung
On 4/4/20 9:16 AM, Peter Levart wrote: Hi Mandy, Just another observation of the code in InnerClassLambdaMetafactory... For the useImplMethodHandle case you generate the protectedImplMethod static field to hold the MH and a static setter method:     mv =

Review Request: 8238358: Implementation of JEP 371: Hidden Classes

2020-03-26 Thread Mandy Chung
Please review the implementation of JEP 371: Hidden Classes. The main changes are in core-libs and hotspot runtime area.  Small changes are made in javac, VM compiler (intrinsification of Class::isHiddenClass), JFR, JDI, and jcmd.  CSR [1]has been reviewed and is in the finalized state (see

Re: Review Request 8238195: Lookup::defineClass should link the class to match the specification

2020-04-02 Thread Mandy Chung
On 4/2/20 1:07 AM, Chris Hegarty wrote: On 1 Apr 2020, at 19:43, Mandy Chung wrote: .. Webrev at: http://cr.openjdk.java.net/~mchung/jdk15/webrevs/8238195/webrev.00/ LGTM. Trivially, you could update the copyright year range on the test. Updated. thanks Mandy

Re: Review Request: 8238358: Implementation of JEP 371: Hidden Classes

2020-04-03 Thread Mandy Chung
Hi Peter, I added a JBS comment [1] to describe this special case trying to put the story together (let me know if it needs more explanation). More comment inline below. On 4/3/20 4:40 AM, Peter Levart wrote: Ok, I think I found one such use-case. In the following example: package test;

Re: Review Request: 8238358: Implementation of JEP 371: Hidden Classes

2020-03-27 Thread Mandy Chung
the sentence you commented on: On 3/27/20 1:18 PM, Mandy Chung wrote: MethodHandles.java — 1786  * (Given the {@code Lookup} object returned this method, its lookup class 1787  * is a {@code Class} object for which {@link Class#getName()} returns a string 1788

Re: Review Request: 8238358: Implementation of JEP 371: Hidden Classes

2020-03-27 Thread Mandy Chung
rget is ready for nestmates or not. I think that you can follow a similar approach here. Thanks, Vicente [1] http://hg.openjdk.java.net/jdk/jdk/rev/2f2af62dfac7 On 3/27/20 12:29 PM, Mandy Chung wrote: Hi Jan, Good point.  The javac change only applies to JDK 15 and later and the lambd

Re: Review Request: 8238358: Implementation of JEP 371: Hidden Classes

2020-03-27 Thread Mandy Chung
On 3/27/20 11:59 AM, Paul Sandoz wrote: Hi Mandy, Very thorough, bravo! Thanks. Minor suggestions below. Paul. MethodHandleNatives.java — 142 143 /** 144 * Flags for Lookup.ClassOptions 145 */ 146 static final int 147

Re: Review Request: 8238358: Implementation of JEP 371: Hidden Classes

2020-03-27 Thread Mandy Chung
On 3/27/20 4:01 PM, David Holmes wrote: Hi Mandy, On 28/03/2020 8:29 am, Mandy Chung wrote: Hi Vicente, hasNestmateAccess is about VM supports static nestmates on JDK release  >= 11. However this is about javac --release 14 and the compiled classes may run on JDK 14 that lam

Re: Review Request: 8238358: Implementation of JEP 371: Hidden Classes

2020-03-27 Thread Mandy Chung
On 3/27/20 5:00 AM, Remi Forax wrote: Hi Mandy, in ReflectionFactory, why in the case of a constructor the check to the anonymous class is removed ? Good catch.  Fixed in BytecodeGenerator, the comment "// bootstrapping issue if using condy" can be promoted on top of clinit, because i ask

Re: Review Request: 8238358: Implementation of JEP 371: Hidden Classes

2020-03-31 Thread Mandy Chung
the new inline @jvms tag. Thanks Mandy [1] https://bugs.openjdk.java.net/browse/JDK-8238359 On 3/26/20 4:57 PM, Mandy Chung wrote: Please review the implementation of JEP 371: Hidden Classes. The main changes are in core-libs and hotspot runtime area.  Small changes are made in javac, VM

Re: RFR (S): 8241947: Minor comment fixes for system property handling

2020-03-31 Thread Mandy Chung
On 3/31/20 7:56 AM, Langer, Christoph wrote: Hi, please review a small fix that updates two comments. The first one, in make/autoconf/spec.gmk.in, is probably quite old. It talks about handling of a property "vm.vendor" in VersionProps.java.template. However, there is no property

Re: Review Request: 8238358: Implementation of JEP 371: Hidden Classes

2020-03-31 Thread Mandy Chung
`, it could be a class or interface. Although isHiddenClass seems clearer, I'm okay to rename it to `isHidden`. Mandy On 3/31/20 11:06 AM, Mandy Chung wrote: This patch addresses Joe's feedback on the CSR [1]: http://cr.openjdk.java.net/~mchung/valhalla/webrevs/hidden-classes/webrev.03-delta-jdarcy

(XS) Review Request JDK-8241964: Clean up java.lang.Class javadoc

2020-03-31 Thread Mandy Chung
 This patch cleans up the javadoc in java.lang.Class and replaces "this object" and "this class object" with "this {@code Class} object" Webrev at: http://cr.openjdk.java.net/~mchung/jdk15/webrevs/8241964/webrev.00/ thanks Mandy

Re: (XS) Review Request JDK-8241964: Clean up java.lang.Class javadoc

2020-03-31 Thread Mandy Chung
Thanks Joe for the quick review. Mandy On 3/31/20 4:22 PM, Joe Darcy wrote: Looks fine Mandy; thanks for doing this cleanup, -Joe On 3/31/2020 4:15 PM, Mandy Chung wrote:  This patch cleans up the javadoc in java.lang.Class and replaces "this object" and "th

Re: Review Request: 8238358: Implementation of JEP 371: Hidden Classes

2020-03-29 Thread Mandy Chung
need tests for these cases. I think these are all things that can be addressed later. Good catch.  I have created a subtask under JDK-8230502:    https://bugs.openjdk.java.net/browse/JDK-8230502 I still need to look over the JVMTI tests. Thanks Mandy thanks, Chris On 3/26/20 4:57 PM, Mandy Chung wro

Re: Review Request: 8238358: Implementation of JEP 371: Hidden Classes

2020-03-30 Thread Mandy Chung
This is the patch to keep the JDK 14 behavior if target release to 14 (thanks to Jan for helping making change in javac to get the tests working) http://cr.openjdk.java.net/~mchung/valhalla/webrevs/8171335/webrev-javac-target-release-14/ Mandy On 3/27/20 9:29 AM, Mandy Chung wrote: Hi Jan

Re: Review Request: 8238358: Implementation of JEP 371: Hidden Classes

2020-03-27 Thread Mandy Chung
, Regarding the javac changes - should those be switched on/off depending the Target? Or, if one compiles with e.g. --release 14, will the newly generated output still work on JDK 14? Jan On 27. 03. 20 0:57, Mandy Chung wrote: Please review the implementation of JEP 371: Hidden Classes. The main

Re: RFR (S): 8241947: Minor comment fixes for system property handling

2020-04-01 Thread Mandy Chung
(). So what do you think of this version: http://cr.openjdk.java.net/~clanger/webrevs/8241947.1/ <http://cr.openjdk.java.net/~clanger/webrevs/8241947.1/> ? Thanks Christoph *From:* Mandy Chung *Sent:* Dienstag, 31. März 2020 19:34 *To:* Langer, Christoph ; core-libs-dev@openjdk.java.ne

Review Request 8238195: Lookup::defineClass should link the class to match the specification

2020-04-01 Thread Mandy Chung
The spec of `Lookup::defineClass` specifies to throw linkage error including `VerifyError` and JDK-8238358 fixes the implementation to match the spec [1]. This patch updates the spec to make it clear as proposed in the CSR:https://bugs.openjdk.java.net/browse/JDK-8240338.  Also add a new

Re: Review Request: 8238358: Implementation of JEP 371: Hidden Classes

2020-04-01 Thread Mandy Chung
On 4/1/20 7:26 AM, Alan Bateman wrote: Maybe I missed it going by, but why is the isHidden check in the field offset methods on sun.misc.Unsafe rather than the internal Unsafe? The reflection and VarHandle implementation uses jdk.internal.misc.Unsafe to do field access (both read and

Re: RFR 8015413 Reformat line in Class.privateGetPublicFields() to be more debugger-friendly

2020-04-01 Thread Mandy Chung
IMHO the debugger should do a better job for us rather than we change the code to work for the debugger. Mandy On 3/31/20 12:02 AM, Vipin Mv1 wrote: Hi All, Please find the below changes for the issue https://bugs.openjdk.java.net/browse/JDK-8015413 diff -r 53568400fec3

  1   2   3   4   5   6   7   8   9   10   >