Re: JDK 15 RFR of JDK-8230771: Remove terminally deprecated constructors in java.base

2019-12-09 Thread Mandy Chung
Looks good. Mandy On 12/8/19 10:58 AM, Joe Darcy wrote: Hello, Please review this small API changes for JDK 15:     JDK-8230771: Remove terminally deprecated constructors in java.base     CSR: https://bugs.openjdk.java.net/browse/JDK-8235548     webrev:

Re: RFR (M) 8223261 "JDK-8189208 followup - remove JDK_GetVersionInfo0 and the supporting code"

2019-12-07 Thread Mandy Chung
Looks good. Mandy On 12/7/19 5:34 AM, gerard ziemski wrote: Hi all, Please review this revision 2 of a cleanup, where we remove JDK_GetVersionInfo0 and related code, since we can get build versions directly from within the VM itself. The rev2 differs from rev1 in that it's simpler due to

RFR JDK-8235351: Lookup::unreflect should bind with the original caller independent of Method's accessible flag

2019-12-05 Thread Mandy Chung
When unreflecting on a Method for caller-sensitive method, the produced MethodHandle should bind with the original caller lookup independent of the accessible flag. Webrev:   http://cr.openjdk.java.net/~mchung/jdk14/8235351/webrev.00/ thanks Mandy

Re: JDK 14 RFR of JDK-8235369: "Class.toGenericString need to be updated for records"

2019-12-05 Thread Mandy Chung
+1 Mandy On 12/5/19 10:08 AM, Joe Darcy wrote: Hello, Please review this small cleanup to records support in java.lang.Class:     JDK-8235369: "Class.toGenericString need to be updated for records"     CSR: https://bugs.openjdk.java.net/browse/JDK-8235428     webrev:

Re: RFR: JEP 367: Remove the Pack200 Tools and API

2019-12-05 Thread Mandy Chung
On 12/5/19 12:41 AM, Alan Bateman wrote: On 05/12/2019 08:16, Henry Jen wrote: Hi, Updated webrev[1] reflect comments since last webrev. Vicente had done all the heavy-lifting and hand over to me to finish up. Changes to symbols is reverted, as we expect that only need to be updated in

Re: RFR 8235359: Simplify method Class.getRecordComponents()

2019-12-05 Thread Mandy Chung
Looks good. Mandy On 12/5/19 6:36 AM, Harold Seigel wrote: Hi, Please review this small change to simplify Class.getRecordComponents() by changing the return type of getRecordComponents0() to RecordComponent[]. Open Webrev:

Re: RFR(XS): 8234696: tools/jlink/plugins/VendorInfoPluginsTest.java times out

2019-12-03 Thread Mandy Chung
Hi Arno, On 12/3/19 1:13 AM, Zeller, Arno wrote: Hi all, could someone please review this small improvement for the test VendorInfoPluginsTest.java? The change avoids writing a core file and might reduce the memory footprint . JBS: https://bugs.openjdk.java.net/browse/JDK-8234696 Webrev:

Re: RFR: JEP 359: Records (Preview) (full code)

2019-12-03 Thread Mandy Chung
Hi Vicente, I reviewed jvm.h, jvm.cpp, and the changes in java.base but only skimmed on the serialization change from this version: http://cr.openjdk.java.net/~vromero/records.review/all_code/webrev.01/ Class::getRecordComponents    - JVM_GetRecordComponents creates a new RecordComponent

Re: RFR: JEP 367: Remove the Pack200 Tools and API

2019-11-25 Thread Mandy Chung
On 11/22/19 1:30 PM, Vicente Romero wrote: [1] http://cr.openjdk.java.net/~vromero/8234542/webrev.03/ make/lib/Lib-jdk.pack.gmk should be removed. make/scripts/compare_exceptions.sh.incl     Should ./lib/libunpack.so also be removed? make/scripts/compare.sh     line 535-543 invokes

Re: Looking for Sponsor: JDK-8229959 Convert proxy class to use constant dynamic

2019-11-22 Thread Mandy Chung
On 11/22/19 2:05 PM, Johannes Kuhn wrote: On 22.11.2019 22:41, Remi Forax wrote: i wonder if some codes in the wild rely on that ? I don't think some code does, but you never know. There is perhaps a better way, as part of the branch nestmates of valhalla, Mandy has added a way to transfer

Re: (XS) RFR JDK-8233956: MethodHandles.dropArguments javadoc lists parameters in wrong order

2019-11-22 Thread Mandy Chung
Thanks for the prompt review. Mandy On 11/22/19 11:26 AM, Jonathan Gibbons wrote: Looks good to me. -- Jon On 11/22/19 11:17 AM, Mandy Chung wrote: This patch reorders `@param pos` of MethodHandles.dropArguments matching the method signature. It'd be nice if javadoc generates

(XS) RFR JDK-8233956: MethodHandles.dropArguments javadoc lists parameters in wrong order

2019-11-22 Thread Mandy Chung
This patch reorders `@param pos` of MethodHandles.dropArguments matching the method signature. It'd be nice if javadoc generates with the ordered list of @params matching the method signature (I created JDK-8234682). diff --git

Re: RFR: JEP 367: Remove the Pack200 Tools and API

2019-11-21 Thread Mandy Chung
CSR needs to mention that jar -n option is removed.  I made minor edit to the CSR to state that jdk.pack module is removed. Mandy On 11/21/19 2:22 PM, Vicente Romero wrote: please wait, I found some additional dependencies on module jdk.pack, will submit another webrev, sorry Vicente On

Re: RFR: JEP 367: Remove the Pack200 Tools and API

2019-11-20 Thread Mandy Chung
(bcc jdk-dev.   The review can continue on core-libs-dev) Hi Vicente, The following files should also be removed.    make/launcher/Launcher-jdk.pack.gmk test/jdk/java/util/jar/Pack200/* The following files reference pack200 in its comment.  I'm not sure if they need update or not.

Re: RFR JDK-8233527: Update Lookup::hasPrivateAccess and Lookup::defineClass spec w.r.t. full power lookup

2019-11-15 Thread Mandy Chung
On 11/15/19 1:32 AM, Peter Levart wrote: Hi Mandy, http://cr.openjdk.java.net/~mchung/jdk14/8233527/webrev.03/ In Lookup's findBoundCallerClass and checkSecurityManager the javadoc is still talking about private access only although the checks are now made against full privilege access.

Re: RFR 8233272 : The Class.forName specification should be updated to match the long-standing implementation with respect to class linking

2019-11-15 Thread Mandy Chung
On 11/15/19 10:38 AM, Brent Christian wrote: That sounds good.  Test updated here: http://cr.openjdk.java.net/~bchristi/8233272/webrev-04/ Looks good.  Minor: an additional check to consider is to check if NCDFE's cause whose message contains  "MissingClass" just to be sure.   No new

Re: RFR JDK-8233527: Update Lookup::hasPrivateAccess and Lookup::defineClass spec w.r.t. full power lookup

2019-11-14 Thread Mandy Chung
On 11/14/19 5:04 PM, David Holmes wrote: Hi Mandy, On 15/11/2019 10:51 am, Mandy Chung wrote: On 11/14/19 8:04 AM, Mandy Chung wrote: On 11/14/19 2:33 AM, Alan Bateman wrote: On 14/11/2019 04:57, Mandy Chung wrote: Updated in place: http://cr.openjdk.java.net/~mchung/jdk14/8233527

Re: RFR JDK-8233527: Update Lookup::hasPrivateAccess and Lookup::defineClass spec w.r.t. full power lookup

2019-11-14 Thread Mandy Chung
On 11/14/19 8:04 AM, Mandy Chung wrote: On 11/14/19 2:33 AM, Alan Bateman wrote: On 14/11/2019 04:57, Mandy Chung wrote: Updated in place: http://cr.openjdk.java.net/~mchung/jdk14/8233527/webrev.02/ http://cr.openjdk.java.net/~mchung/jdk14/8233527/specdiff/ The addition

Re: RFR 8233272 : The Class.forName specification should be updated to match the long-standing implementation with respect to class linking

2019-11-14 Thread Mandy Chung
On 11/14/19 4:42 PM, David Holmes wrote: On 15/11/2019 10:33 am, Brent Christian wrote: On 11/14/19 4:12 PM, David Holmes wrote: On 15/11/2019 9:58 am, Brent Christian wrote: http://cr.openjdk.java.net/~bchristi/8233272/webrev-03/ Test is fine. Just one note/clarification:   63

Re: RFR 8233272 : The Class.forName specification should be updated to match the long-standing implementation with respect to class linking

2019-11-14 Thread Mandy Chung
On 11/13/19 10:37 AM, Brent Christian wrote: Hi, Recently, the 2-arg and 3-arg Class.forName() methods were updated[1] to perform class linking, per the specification. However this change had to be reverted[2]. Instead, let's clarify the Class.forName() spec not to guarantee linking

Re: RFR JDK-8233527: Update Lookup::hasPrivateAccess and Lookup::defineClass spec w.r.t. full power lookup

2019-11-14 Thread Mandy Chung
On 11/14/19 2:33 AM, Alan Bateman wrote: On 14/11/2019 04:57, Mandy Chung wrote: Updated in place: http://cr.openjdk.java.net/~mchung/jdk14/8233527/webrev.02/ http://cr.openjdk.java.net/~mchung/jdk14/8233527/specdiff/ The addition of hasFullPrivilegeAccess looks okay and probably

Re: RFR JDK-8233527: Update Lookup::hasPrivateAccess and Lookup::defineClass spec w.r.t. full power lookup

2019-11-13 Thread Mandy Chung
On 11/13/19 11:45 AM, Johannes Kuhn wrote: The two bullets about the caller lookup object must have MODULE and PRIVATE are important explanation for it to require such access. Maybe I can add a bullet to say "The caller lookup object must have full privilege access" and then move those two

Re: RFR JDK-8233527: Update Lookup::hasPrivateAccess and Lookup::defineClass spec w.r.t. full power lookup

2019-11-13 Thread Mandy Chung
On 11/13/19 11:45 AM, Johannes Kuhn wrote: This is a good idea, because full privilege access is a new concept, and should be mentioned at the places where it is used. To clarify, "full privilege access" (aka "full power") is not really a new concept. Module access was introduced in

Re: RFR (M) 8223261 "JDK-8189208 followup - remove JDK_GetVersionInfo0 and the supporting code"

2019-11-13 Thread Mandy Chung
On 11/13/19 10:50 AM, gerard ziemski wrote: Hi all, Please review this cleanup, where we remove JDK_GetVersionInfo0 and related code, since we can get build versions directly from within the VM itself: I'm including core-libs and awt in this review because the proposed fix touches their

Re: RFR JDK-8233527: Update Lookup::hasPrivateAccess and Lookup::defineClass spec w.r.t. full power lookup

2019-11-13 Thread Mandy Chung
On 11/13/19 6:10 AM, Johannes Kuhn wrote: On 11.11.2019 22:23, Mandy Chung wrote: This is a follow-up of JDK-8226916. Lookup::hasPrivateAccess intends to test if this lookup is a full-power lookup; that is created by the original caller class calling MethodHandles::lookup. The current

Re: RFR JDK-8233527: Update Lookup::hasPrivateAccess and Lookup::defineClass spec w.r.t. full power lookup

2019-11-12 Thread Mandy Chung
/webrev.03/ http://cr.openjdk.java.net/~mchung/jdk14/8233527/specdiff/ CSR has been updated as well. Thanks Mandy On 11/11/19 1:23 PM, Mandy Chung wrote: This is a follow-up of JDK-8226916. Lookup::hasPrivateAccess intends to test if this lookup is a full-power lookup; that is created

Re: RFR JDK-8233527: Update Lookup::hasPrivateAccess and Lookup::defineClass spec w.r.t. full power lookup

2019-11-12 Thread Mandy Chung
On 11/12/19 4:56 AM, Alan Bateman wrote: On 11/11/2019 21:23, Mandy Chung wrote: This is a follow-up of JDK-8226916. Lookup::hasPrivateAccess intends to test if this lookup is a full-power lookup; that is created by the original caller class calling MethodHandles::lookup. The current

RFR JDK-8233527: Update Lookup::hasPrivateAccess and Lookup::defineClass spec w.r.t. full power lookup

2019-11-11 Thread Mandy Chung
This is a follow-up of JDK-8226916. Lookup::hasPrivateAccess intends to test if this lookup is a full-power lookup; that is created by the original caller class calling MethodHandles::lookup. The current specification for Lookup::hasPrivateAccess returns true if the lookup modes contain

Re: RFR: 8231863: Crash if classpath is read from @argument file and the main gets option argument

2019-11-11 Thread Mandy Chung
On 11/11/19 8:53 AM, Henry Jen wrote: Thanks Alan and Mandy for the review. I am guessing the Alan’s preference to use Files.write(aFilePath, lines) is to avoid extra String.join operation, which I would too. The current version with byte[] is more flexible but we most likely won't need it

Re: RFR: 8231863: Crash if classpath is read from @argument file and the main gets option argument

2019-11-09 Thread Mandy Chung
The patch looks fine to me as well. As for the test, perhaps adding a new createAFile(File aFile, List lines, boolean hasTrailingBlankLine) in TestHelper instead may help avoid any confusion. Mandy On 11/8/19 7:43 AM, Mat Carter wrote: Hi Alan The method you propose:

Re: RFR: JDK-8233636: Make jpackage an incubator and remove tool provider implementation

2019-11-07 Thread Mandy Chung
On 11/7/19 1:45 PM, Andy Herrick wrote: Please review the jpackage fix for bug [1] at [2]. This is a fix for the JDK-8200758-branch branch of the open sandbox repository (jpackage). This changes the module name, and base package name from jdk.jpackage to jdk.incubator.jpackage. This

Re: RFR: 8233291: [TESTBUG] tools/jlink/plugins/VendorInfoPluginsTest.java fails with debug VMs

2019-11-03 Thread Mandy Chung
-- Please review it and give me some advice. Thanks a lot. Best regards, Jie On 2019/11/2 上午1:15, Mandy Chung wrote: Why do you need to check if it contains "debug"? Because System.getProperty("jdk.debug") will return "release" for release

Re: RFR: 8233291: [TESTBUG] tools/jlink/plugins/VendorInfoPluginsTest.java fails with debug VMs

2019-11-01 Thread Mandy Chung
On 10/31/19 9:08 PM, Jie Fu wrote: Hi Mandy, Thanks for your review and nice suggestion. Updated: http://cr.openjdk.java.net/~jiefu/8233291/webrev.01/ Why do you need to check if it contains "debug"?  I would expect it's simply empty string or the "jdk.debug" system property value appended

Re: RFR 8233091 : Backout JDK-8212117: Class.forName loads a class but not linked if class is not initialized

2019-10-31 Thread Mandy Chung
This looks good to me. Mandy On 10/31/19 3:50 PM, Brent Christian wrote: Hi, Please review my change to backout JDK-8212117: http://cr.openjdk.java.net/~bchristi/8233091/webrev-revert-01/ Background: A couple months ago, JDK-8212117[1] was fixed[2].  It changed the behavior of the 2-arg

Re: RFR: 8233291: [TESTBUG] tools/jlink/plugins/VendorInfoPluginsTest.java fails with debug VMs

2019-10-31 Thread Mandy Chung
Hi Jie, On 10/30/19 11:13 PM, Jie Fu wrote: Hi all, May I get reviews for the small fix for the failure of VendorInfoPluginsTest.java in debug VMs? JBS:    https://bugs.openjdk.java.net/browse/JDK-8233291 Webrev: http://cr.openjdk.java.net/~jiefu/8233291/webrev.00/ Thanks for fixing this.

Re: 8205132: Remove Thread.countStackFrames()

2019-10-29 Thread Mandy Chung
On 10/29/19 9:26 AM, Alan Bateman wrote: On 23/10/2019 08:25, Alan Bateman wrote: Thread::countStackFrames has been deprecated for 20+ years and has been marked for-removal since Java SE 9. I'd like to remove it for Java SE 14. It's was never a well-defined method and I've been unable to

Re: RFR: JDK-8232773: ClassLoading Debug Output for Specific Classes

2019-10-29 Thread Mandy Chung
Hi Adam, It'd be useful to provide a few typical scenarios that customers run into. That would give better understanding on the problem "hard to diagnose why a given class was not loaded" and help the consideration on alternatives.   The debug output could produce lots of trace output but

Re: Review Request JDK-8173975: Lookup::in should not allow target class be primitive or array class

2019-10-29 Thread Mandy Chung
On 10/15/19 2:26 AM, Alan Bateman wrote: On 14/10/2019 23:34, Mandy Chung wrote: MethodHandles::lookup produces a Lookup object on the caller class. The original intention for a Lookup object whose lookup class is always a non-array and non-primitive class. MethodHandles::privateLookupIn

Re: RFR: 8232864: Classes generated by GenerateJLIClassesPlugin.java are not reproducable

2019-10-29 Thread Mandy Chung
your refactoring is a nice improvement. Updated: http://cr.openjdk.java.net/~jiefu/8232864/webrev.03/ It seems much better now. Testing:  - make test TEST="tier1 tier2 tier3" CONF=release Thanks a lot. Best regards, Jie On 2019/10/26 上午3:29, Mandy Chung wrote: On 10/23/19 11:21

Re: RFR: 8232864: Classes generated by GenerateJLIClassesPlugin.java are not reproducable

2019-10-25 Thread Mandy Chung
On 10/23/19 11:21 PM, Jie Fu wrote: Hi Mandy, Updated: http://cr.openjdk.java.net/~jiefu/8232864/webrev.02/ It seems better now. It's better while I think this can be further simplified. line 101 to 217 involves iterating on the default sets/map and then create a new TreeSet and TreeMap

Re: 8205132: Remove Thread.countStackFrames()

2019-10-24 Thread Mandy Chung
There is cost to examine the stack frames.  The StackWalker API allows you to fetch the stack frames one batch at a time to avoid unnecessary deoptimization to frames that are not traversed.  In addition,it can specify to include/exclude the hidden frames and/or reflection

Re: RFR: 8232864: Classes generated by GenerateJLIClassesPlugin.java are not reproducable

2019-10-23 Thread Mandy Chung
/ The JLinkReproducibleTest had been extended to test the case when the default trace file is not present. If you're OK with the change, could you please sponsor it? Thanks a lot. Best regards, Jie On 2019/10/24 上午2:37, Mandy Chung wrote: Can you also extend JLinkReproducibleTest to add

Re: RFR: 8232864: Classes generated by GenerateJLIClassesPlugin.java are not reproducable

2019-10-23 Thread Mandy Chung
On 10/23/19 12:16 AM, Jie Fu wrote: Hi all, JBS:    https://bugs.openjdk.java.net/browse/JDK-8232864 Webrev: http://cr.openjdk.java.net/~jiefu/8232864/webrev.00/ speciesTypes, invokerTypes, callSiteTypes and dmhMethods are sorted when the trace config is present (done in readTraceConfig).

Re: 8205132: Remove Thread.countStackFrames()

2019-10-23 Thread Mandy Chung
On 10/23/19 12:25 AM, Alan Bateman wrote: Thread::countStackFrames has been deprecated for 20+ years and has been marked for-removal since Java SE 9. I'd like to remove it for Java SE 14. It's was never a well-defined method and I've been unable to find anything that uses it. The

Re: 14 RFR (M) 8232080: jlink plugins for vendor information and run-time options

2019-10-22 Thread Mandy Chung
On 10/22/19 3:37 PM, mark.reinh...@oracle.com wrote: 2019/10/22 14:12:18 -0700, mandy.ch...@oracle.com: On 10/21/19 8:22 PM, mark.reinh...@oracle.com wrote: RFE: https://bugs.openjdk.java.net/browse/JDK-8232080 CSR: https://bugs.openjdk.java.net/browse/JDK-8232753 Webrev:

Re: RFR: 8232613: Move Object.registerNatives into HotSpot

2019-10-22 Thread Mandy Chung
On 10/22/19 8:03 AM, Claes Redestad wrote: http://cr.openjdk.java.net/~redestad/8232613/open.03 Looks good to me. Filed and drafted a CSR: https://bugs.openjdk.java.net/browse/JDK-8232801 I think it'd be good to include the IAE message as an example. Is there a CSR that documents the

Re: 14 RFR (M) 8232080: jlink plugins for vendor information and run-time options

2019-10-22 Thread Mandy Chung
On 10/21/19 8:22 PM, mark.reinh...@oracle.com wrote: RFE: https://bugs.openjdk.java.net/browse/JDK-8232080 CSR: https://bugs.openjdk.java.net/browse/JDK-8232753 Webrev: https://cr.openjdk.java.net/~mr/rev/8232080/ Looks good to me.  One minor comment: AddResourcePlugin isn't really a

Re: 8231602: Deprecate Thread.suspend/resume for removal

2019-10-22 Thread Mandy Chung
Looks fine to me. Mandy On 10/22/19 11:25 AM, Alan Bateman wrote: I brought up the issue of deprecating, for removal, Thread.suspend/resume a few weeks ago [1]. Mark said "Got for it", no other replies or objections. Here's the webrev with the changes to deprecate-for-removal

Re: Review Request JDK-8232617: Update the outdated code comments in java.lang.System class

2019-10-21 Thread Mandy Chung
On 10/20/19 3:50 PM, David Holmes wrote: On 19/10/2019 10:38 am, Mandy Chung wrote: thanks. I think "separated from" reads right to me.  I will leave it as is. I think "separately" or "separate" seems more correct here. And if we're picking on gra

Re: Review Request JDK-8232617: Update the outdated code comments in java.lang.System class

2019-10-18 Thread Mandy Chung
thanks. I think "separated from" reads right to me.  I will leave it as is. Mandy On 10/18/19 2:46 PM, Brent Christian wrote: Looks fine.  You might consider s/separated/separately/ . -Brent On 10/18/19 1:56 PM, Mandy Chung wrote: A trivial doc fix: diff --git a/src/java.

Review Request JDK-8232617: Update the outdated code comments in java.lang.System class

2019-10-18 Thread Mandy Chung
A trivial doc fix: diff --git a/src/java.base/share/classes/java/lang/System.java b/src/java.base/share/classes/java/lang/System.java --- a/src/java.base/share/classes/java/lang/System.java +++ b/src/java.base/share/classes/java/lang/System.java @@ -94,10 +94,8 @@ public final class System {

Review Request JDK-8173975: Lookup::in should not allow target class be primitive or array class

2019-10-14 Thread Mandy Chung
MethodHandles::lookup produces a Lookup object on the caller class. The original intention for a Lookup object whose lookup class is always a non-array and non-primitive class. MethodHandles::privateLookupIn and Lookup::in are the two other ways that can produce a new Lookup object and they

Re: RFR: 8231584: Deadlock with ClassLoader.findLibrary and System.loadLibrary call

2019-10-11 Thread Mandy Chung
On 10/11/19 2:35 AM, Anton Kozlov wrote: On 11.10.2019 00:28, Mandy Chung wrote: Since the method throws Exception, this try-catch block is not needed. The start year of the copyright in the new test files should be 2019. Thanks! I preserved Amazon's copyright year as 2018, as I derived

Re: RFR: 8231584: Deadlock with ClassLoader.findLibrary and System.loadLibrary call

2019-10-10 Thread Mandy Chung
On 10/10/19 9:54 AM, Anton Kozlov wrote: Updated review is: http://cr.openjdk.java.net/~akozlov/8231584/webrev.04/ Nit: 112 try { 113 loader = new TestClassLoader(); 114 } catch (MalformedURLException e) { 115 throw new RuntimeException(e); 116

Re: RFR: 8231584: Deadlock with ClassLoader.findLibrary and System.loadLibrary call

2019-10-09 Thread Mandy Chung
On 10/9/19 9:24 AM, Anton Kozlov wrote: Updated webrev with fixes: http://cr.openjdk.java.net/~akozlov/8231584/webrev.03/ I like this version.  Some minor comments: 2014 static synchronized void initLibraryPaths() { This does not need synchronized since it's still during phase 1 before

Re: RFR: JDK-8229871: Improve performance of Method.copy() and leafCopy()

2019-10-08 Thread Mandy Chung
On 10/8/19 6:31 AM, Claes Redestad wrote: Hi, webrev.02 looks good to me. webrev.02 looks fine to me as well. I think the performance results makes sense since avoiding a volatile store (and the potentially expensive store barriers this involves) should be the main benefit. Adding a

Re: RFR: 8231584: Deadlock with ClassLoader.findLibrary and System.loadLibrary call

2019-10-07 Thread Mandy Chung
On 10/6/19 9:23 AM, Anton Kozlov wrote: Hi Mandy, On 02.10.2019 01:08, Anton Kozlov wrote:> On 30.09.2019 21:18, Mandy Chung wrote: Skimming through the implementation, it seems to me that the Runtime::loadLibrary0 does not need to be synchronized. ClassLoader::loadLibrary0 should ens

Re: RFR: 8231584: Deadlock with ClassLoader.findLibrary and System.loadLibrary call

2019-10-02 Thread Mandy Chung
On 10/1/19 3:08 PM, Anton Kozlov wrote: New intermediate webrev: (while investigating) http://cr.openjdk.java.net/~akozlov/8231584/webrev.01/ Just on the test, instead of storing Target and Target2 classfile bytes in the source, the test can use jdk.test.lib.compiler.CompilerUtils to

Re: RFR: 8231584: Deadlock with ClassLoader.findLibrary and System.loadLibrary call

2019-09-30 Thread Mandy Chung
I will need to look at this closer.   Skimming through the implementation, it seems to me that the Runtime::loadLibrary0 does not need to be synchronized.   ClassLoader::loadLibrary0 should ensure that a native library of a given name is loaded and registered only once.   Can you explain why

Re: RFR 8221623 : Add StackWalker micro benchmarks to jdk repo

2019-09-24 Thread Mandy Chung
On 9/24/19 1:01 PM, Brent Christian wrote: http://cr.openjdk.java.net/~bchristi/8221623/webrev11/ Looks okay.  Thanks for doing this. Mandy

Re: RFR 8221623 : Add StackWalker micro benchmarks to jdk repo

2019-09-23 Thread Mandy Chung
On 9/19/19 12:08 PM, Brent Christian wrote: Hi, Daniel http://cr.openjdk.java.net/~bchristi/8221623/webrev09-loggerPerThread/ I think doing the measurement for one of these would be adequate. StackWalkBench.forEach_AllOpts StackWalkBench.forEach_DefaultOpts

Re: RFR 8221623 : Add StackWalker micro benchmarks to jdk repo

2019-09-19 Thread Mandy Chung
benchmarks using 'java -jar path/to/benchmark.jar'. It may be worth looking into updating the make target to setup more practical JMH runtimes by default.  But I don't think this an issue particular the new benchmarks. Thanks, -Brent On 9/18/19 7:33 PM, Mandy Chung wrote: Hi B

Re: RFR 8221623 : Add StackWalker micro benchmarks to jdk repo

2019-09-18 Thread Mandy Chung
Hi Brent, $ make test TEST="micro:java.lang.StackWalkBench" It took very long that I killed the job.  Does this happen to you? Mandy On 9/13/19 3:07 PM, Brent Christian wrote: Hi, Please review these StackWalker and Throwable benchmarks for addition into the JDK microbenchmarks. Bug:

Re: RFR: 8230768: Arrays of SoftReferences in MethodTypeForm should not be @Stable

2019-09-18 Thread Mandy Chung
Looks fine to me. Mandy On 9/18/19 11:05 AM, Claes Redestad wrote: Hi, please review this patch to remove @Stable from two arrays in MethodTypeForm. These were marked as @Stable in an earlier incarnation of the code when the contents was not SoftReference, but @Stable does not make sense for

Re: RFR 8230937 : Update bugid in ProblemList for vmTestbase/nsk/jdb/eval/eval001/eval001.java

2019-09-12 Thread Mandy Chung
+1 Mandy On 9/12/19 4:02 PM, Brent Christian wrote: Hi, From the bug report: "The ProblemList indicates that vmTestbase/nsk/jdb/eval/eval001/eval001.java was added due to JDK-8212117. That bug has been fixed, but this test still fails. That line in the ProblemList should instead use

RFR JDK-8229785: MethodType::fromMethodDescriptorString should require security permission if loader is null

2019-09-09 Thread Mandy Chung
MethodType::fromMethodDescriptorString default to use the system class loader in resolving classes per the given descriptor string if the loader parameter is null.  Since this method accesses the system class loader on behalf of the caller, it should do a security permission check as

Re: RFR: 8230301: Re-examine hardcoded defaults in GenerateJLIClassesPlugin

2019-09-06 Thread Mandy Chung
On 9/6/19 6:29 PM, Claes Redestad wrote: The original default hardcoded list includes many more forms. What is the diff of the jlink-plugin generated classes before and after this patch? I wonder if we should include a few other method types in HelloClassList like float parameter. 

Re: RFR: 8212117 : Class.forName may return a reference to a loaded but not linked Class

2019-09-06 Thread Mandy Chung
On 9/6/19 2:20 PM, Brent Christian wrote: Update webrev is here: http://cr.openjdk.java.net/~bchristi/8212117/webrev11/ with changes to jvm.h, MethodHandles.java, and Class.java. Looks good. Mandy

Re: RFR: 8230301: Re-examine hardcoded defaults in GenerateJLIClassesPlugin

2019-09-06 Thread Mandy Chung
Hi Claes, The patch looks much straightforward than I expected. The original default hardcoded list includes many more forms.  What is the diff of the jlink-plugin generated classes before and after this patch? I wonder if we should include a few other method types in HelloClassList like

Re: RFR: 8230662: Remove dead code from MethodTypeForm

2019-09-06 Thread Mandy Chung
Looks good.  This is a good simplification. thanks Mandy On 9/5/19 7:41 AM, Claes Redestad wrote: Hi, I noticed some unused methods in java.lang.invoke.MethodTypeForm and ended up with a rather substantial cleanup after pulling that particular thread for a bit:

Re: RFR: 8212117 : Class.forName may return a reference to a loaded but not linked Class

2019-09-05 Thread Mandy Chung
On 9/5/19 5:09 PM, Brent Christian wrote: Updated webrev: http://cr.openjdk.java.net/~bchristi/8212117/webrev10/ jvm.h 349 * Link the 'arg' class (unless ClassForNameDeferLinking is set) I suggest to drop "(unless ...)" phrase because the flag is temporary. jni.cpp The implementation

Re: RFR: 8212117 : Class.forName may return a reference to a loaded but not linked Class

2019-09-05 Thread Mandy Chung
On 9/4/19 2:12 PM, Brent Christian wrote: There is also a CSR[2] in need of review. The javadoc for Lookup::findClass: "In particular, the method first attempts to load the requested class" I think this sentence is no longer needed together with your change.  What about: /** -

Re: RFR: 8230302: GenerateJLIClassesPlugin can generate invalid DirectMethodHandle methods

2019-08-28 Thread Mandy Chung
On 8/28/19 7:38 AM, Claes Redestad wrote: Hi, we currently spin code for a non-sensical invokeVirtual DMH, so let's remove that. Such methods are benign since we won't ever attempt to resolve such nonsensical forms, but surely a waste of space. This patch tightens checks conservatively

Re: RFR: JDK-8229871: Improve performance of Method.copy() and leafCopy()

2019-08-27 Thread Mandy Chung
: From: Kazunori Ogata/Japan/IBM To: Mandy Chung Cc: core-libs-dev@openjdk.java.net Date: 2019/08/21 20:02 Subject: Re: RFR: JDK-8229871: Improve performance of Method.copy() and leafCopy() Hi Mandy, Thank you for reviewing the webrev. I updated it to add a space after "if" and als

Re: Remove exception from sun.rmi.runtime.Log#getSource()

2019-08-23 Thread Mandy Chung
This patch looks okay to me. Mandy On 8/23/19 8:44 AM, Roger Riggs wrote: Hi Philippe, The change looks good, I'll run it through our tests and push if its all ok. (Pending anyone else's comments.) On 8/23/19 10:38 AM, Philippe Marschall wrote: On 22.08.19 19:56, Mandy Chung wrote: Hi

Re: Remove exception from sun.rmi.runtime.Log#getSource()

2019-08-22 Thread Mandy Chung
Hi Philippe, This is a good use of StackWalker.   getSource can simply return StackFrame and avoid the creation of String[]. Stuart will give you better guidance related to RMI testing. I see that test/jdk/sun/rmi/runtime/Log has a few RMI logging tests. RMI tests are in tier3.  You can run

Re: RFR 8207814: (proxy) upgrade the proxy class generator

2019-08-20 Thread Mandy Chung
and unexpected). http://cr.openjdk.java.net/~rriggs/webrev-upgrade-proxy-gen-8207814/ Thanks, Roger On 8/19/19 6:13 PM, Mandy Chung wrote: This is a good idea to explore.  We should keep this patch to focus on switching ASM.I will file a JBS issue for this suggestion. Mandy On 8/19/19 2:19 PM, Remi

Re: RFR: JDK-8229871: Imporve performance of Method.copy() and leafCopy()

2019-08-20 Thread Mandy Chung
Hi Ogata, The patch looks okay.  Nit: please add a space between if and (. About volatile methodAccessor field, I checked the history.  Both fieldAccessor and methodAccessor were started as volatile and the fieldAccessor declaration was updated due to JDK-5044412.   As you observe, I think

Re: RFR 8207814: (proxy) upgrade the proxy class generator

2019-08-19 Thread Mandy Chung
This is a good idea to explore.  We should keep this patch to focus on switching ASM.I will file a JBS issue for this suggestion. Mandy On 8/19/19 2:19 PM, Remi Forax wrote: A follow up should to use constant dynamic (introduce in Java 11) to get the j.l.r.Method object instead of

Re: RFR 8207814: (proxy) upgrade the proxy class generator

2019-08-16 Thread Mandy Chung
Hi Roger, Thanks for doing this.  Replacing ancient bytecode generators makes it much easier to support new features like value types. The diff of ProxyGenerator is not quite right (might be due to the rename?) as some code are unchanged but shown as modified.

Re: RFR JDK-8193325: StackFrameInfo::getByteCodeIndex returns wrong value if bci > 32767

2019-08-15 Thread Mandy Chung
On 8/15/19 12:43 PM, Aleksey Shipilev wrote: On 8/15/19 9:11 PM, Mandy Chung wrote: On 8/15/19 11:59 AM, Aleksey Shipilev wrote: On 8/14/19 9:42 PM, Mandy Chung wrote:     http://cr.openjdk.java.net/~mchung/jdk14/8193325/webrev.05/ Looks good. So, to reiterate, we do not need

Re: RFR JDK-8193325: StackFrameInfo::getByteCodeIndex returns wrong value if bci > 32767

2019-08-15 Thread Mandy Chung
On 8/15/19 11:59 AM, Aleksey Shipilev wrote: On 8/14/19 9:42 PM, Mandy Chung wrote:    http://cr.openjdk.java.net/~mchung/jdk14/8193325/webrev.05/ Looks good. So, to reiterate, we do not need to initialize bci to -1 for StackFrameInfo, because it is not exposed anywhere? No, it is only

Re: RFR JDK-8193325: StackFrameInfo::getByteCodeIndex returns wrong value if bci > 32767

2019-08-14 Thread Mandy Chung
.  Coupling the jdk and jvm like this feels like it's asking for bugs.  I like the simpler implementation that fixes the bug that we have. Thanks, Coleen On 8/13/19 1:49 PM, Mandy Chung wrote: On 8/13/19 9:30 AM, Peter Levart wrote: Usually the StackFrameInfo(s) are consumed as soon

Re: RFR JDK-8193325: StackFrameInfo::getByteCodeIndex returns wrong value if bci > 32767

2019-08-13 Thread Mandy Chung
On 8/13/19 9:30 AM, Peter Levart wrote: Usually the StackFrameInfo(s) are consumed as soon as they are returned from StackWalker API and never assigned to @Stable field. So there's no purpose of @Stable for bci field use. Except documentation. But documentation can be specified in the form

Re: RFR JDK-8193325: StackFrameInfo::getByteCodeIndex returns wrong value if bci > 32767

2019-08-13 Thread Mandy Chung
Peter, Aleksey, On 8/13/19 2:17 AM, Peter Levart wrote: What are you trying to achieve with @Stable annotation? I chose the path of using it for at least documentation purpose that it is initialized once after :)  This field is not final as it's modified by the VM but it's a stable

Re: RFR JDK-8193325: StackFrameInfo::getByteCodeIndex returns wrong value if bci > 32767

2019-08-12 Thread Mandy Chung
/~mchung/jdk14/8193325/webrev.04/ Mandy No need for another review from me. Regards, Fred On Aug 12, 2019, at 16:24, Mandy Chung wrote: Having a second thought, I'm keeping @Stable bci field while zero indicates an invalid BCI that makes it obvious that this field will be updated. VM

Re: RFR JDK-8193325: StackFrameInfo::getByteCodeIndex returns wrong value if bci > 32767

2019-08-12 Thread Mandy Chung
On 8/12/19 3:28 PM, David Holmes wrote: Hi Mandy, On 13/08/2019 6:24 am, Mandy Chung wrote: Having a second thought, I'm keeping @Stable bci field while zero indicates an invalid BCI that makes it obvious that this field will be updated.  VM will set StackFrameInfo::bci to value+1. I

Re: RFR JDK-8193325: StackFrameInfo::getByteCodeIndex returns wrong value if bci > 32767

2019-08-12 Thread Mandy Chung
Having a second thought, I'm keeping @Stable bci field while zero indicates an invalid BCI that makes it obvious that this field will be updated.  VM will set StackFrameInfo::bci to value+1. http://cr.openjdk.java.net/~mchung/jdk14/8193325/webrev.03/ Mandy

Re: RFR JDK-8193325: StackFrameInfo::getByteCodeIndex returns wrong value if bci > 32767

2019-08-12 Thread Mandy Chung
On 8/12/19 11:33 AM, Aleksey Shipilev wrote: Alternatively, we can make bci an int (as Alekey suggests) that does not increase the object size:    http://cr.openjdk.java.net/~mchung/jdk14/8193325/webrev.02/ @Stable seems meaningless here, for the reasons Daniel points out. I believe

Re: RFR JDK-8193325: StackFrameInfo::getByteCodeIndex returns wrong value if bci > 32767

2019-08-12 Thread Mandy Chung
On 8/11/19 9:49 PM, David Holmes wrote: On 11/08/2019 2:50 pm, Mandy Chung wrote: On 8/10/19 12:30 AM, Aleksey Shipilev wrote: On 8/9/19 10:19 PM, Mandy Chung wrote: An earlier version of this patch was reviewed [1] but I didn't get back to explore the other approach.  I rebase the patch

Re: RFR JDK-8193325: StackFrameInfo::getByteCodeIndex returns wrong value if bci > 32767

2019-08-10 Thread Mandy Chung
On 8/10/19 12:30 AM, Aleksey Shipilev wrote: On 8/9/19 10:19 PM, Mandy Chung wrote: An earlier version of this patch was reviewed [1] but I didn't get back to explore the other approach.  I rebase the patch and take out the hotspot change which will be covered by JDK-8229375: http

RFR JDK-8193325: StackFrameInfo::getByteCodeIndex returns wrong value if bci > 32767

2019-08-09 Thread Mandy Chung
An earlier version of this patch was reviewed [1] but I didn't get back to explore the other approach.  I rebase the patch and take out the hotspot change which will be covered by JDK-8229375: http://cr.openjdk.java.net/~mchung/jdk14//8193325/webrev.01 David - can you re-review it? thanks

Re: Are classes generated by LambdaMetafactory special?

2019-08-05 Thread Mandy Chung
One property of a lambda proxy class is to be in the same nest as the caller class as it's logically part of the caller class (as it may access its private member).  VM anonymous class is an interim implementation solution until we provide a way to define a dynamically generated class as a

Re: Are classes generated by LambdaMetafactory special?

2019-08-05 Thread Mandy Chung
This is intentional.  The lambda proxy classes are defined as VM anonymous classes through a JDK internal API Unsafe::defineAnonymousClass.  These generated classes are JDK implementation details that are hidden for any class lookup and not modifiable by JVM TI agent. JDK-8171335 is the RFE

Re: Review Request: JDK-8209005: Lookup.unreflectSpecial fails for default methods when Lookup.findSpecial works

2019-07-31 Thread Mandy Chung
Hi Peter, Daniel On 7/31/19 1:44 AM, Peter Levart wrote: On 7/31/19 9:59 AM, Peter Levart wrote: Hi, I think Daniel is talking about the "dispatch" semantics of unreflectSpecial here, right Daniel? Thanks for clarifying this. The findSpecial / unreflectSpecial is a MethodHandle

Re: Review Request: JDK-8209005: Lookup.unreflectSpecial fails for default methods when Lookup.findSpecial works

2019-07-30 Thread Mandy Chung
not too familiar with the intricacies of MethodHandle. best regards, -- daniel On 26/07/2019 18:41, Mandy Chung wrote: Daniel noticed that `unreflectSpecial` is missing in the "Lookup Factory Methods" section in the class spec.  In fact there are a duplicated `lookup.unreflect(aMe

Re: Review Request: JDK-8228671: Fastdebug VM throws InternalError when publicLookup.in(T) is used to resolve a member

2019-07-27 Thread Mandy Chung
On 7/26/19 11:51 PM, Alan Bateman wrote: On 26/07/2019 22:12, Mandy Chung wrote: Debug VM checks if a class is accessible to the lookup class except if the lookup class is java.lang.Object (which was the lookup class of publicLookup previously). WithJDK-8173978, Lookup::in has changed

Review Request: JDK-8228671: Fastdebug VM throws InternalError when publicLookup.in(T) is used to resolve a member

2019-07-26 Thread Mandy Chung
Debug VM checks if a class is accessible to the lookup class except if the lookup class is java.lang.Object (which was the lookup class of publicLookup previously). WithJDK-8173978, Lookup::in has changed and it can be used to create a new public Lookup on a different lookup class. A quick

Re: Review Request: JDK-8209005: Lookup.unreflectSpecial fails for default methods when Lookup.findSpecial works

2019-07-26 Thread Mandy Chung
et/~mchung/jdk14/8209005/webrev.01/ Mandy On 7/25/19 1:12 PM, Mandy Chung wrote: This patch fixes Lookup.unreflectSpecial to pass the declaring class of Method being unreflected (rather than null) so that it can accurately check if the special caller class is either the lookup class or a supe

Review Request: JDK-8209005: Lookup.unreflectSpecial fails for default methods when Lookup.findSpecial works

2019-07-25 Thread Mandy Chung
This patch fixes Lookup.unreflectSpecial to pass the declaring class of Method being unreflected (rather than null) so that it can accurately check if the special caller class is either the lookup class or a superinterface of the declaring class. Webrev:

<    5   6   7   8   9   10   11   12   13   14   >