Re: RFR: 8229773: Resolve permissions for code source URLs lazily

2019-08-15 Thread Sean Mullan
Hi Claes, I already reviewed an earlier version of this and this is pretty similar. I did have a question about whether the default serialization was ok - did you look into that more? --Sean On 8/15/19 6:03 AM, Claes Redestad wrote: Hi, by resolving permissions for code source URLs

Re: RFR: 8229773: Resolve permissions for code source URLs lazily

2019-08-15 Thread Claes Redestad
Hi Sean, On 2019-08-15 15:07, Sean Mullan wrote: Hi Claes, I already reviewed an earlier version of this and this is pretty similar. I did have a question about whether the default serialization was ok - did you look into that more? ah, yes.. all the constituents are serializable (whether

Re: RFR: 8229773: Resolve permissions for code source URLs lazily

2019-08-15 Thread Claes Redestad
Hi, On 2019-08-15 12:56, Alan Bateman wrote: On 15/08/2019 11:03, Claes Redestad wrote: Hi, by resolving permissions for code source URLs lazily, we can reduce early class loading during bootstrap, which improves footprint, startup and reduces the typical bootstrap dependency graph. Bug:   

Re: RFR: 8229773: Resolve permissions for code source URLs lazily

2019-08-15 Thread Alan Bateman
On 15/08/2019 11:03, Claes Redestad wrote: Hi, by resolving permissions for code source URLs lazily, we can reduce early class loading during bootstrap, which improves footprint, startup and reduces the typical bootstrap dependency graph. Bug:   

RFR: 8229773: Resolve permissions for code source URLs lazily

2019-08-15 Thread Claes Redestad
Hi, by resolving permissions for code source URLs lazily, we can reduce early class loading during bootstrap, which improves footprint, startup and reduces the typical bootstrap dependency graph. Bug:https://bugs.openjdk.java.net/browse/JDK-8229773 Webrev:

Re: RFR: 8229773: Resolve permissions for code source URLs lazily

2019-08-15 Thread Daniel Fuchs
Thanks Claes, Looks good to me too. best regards, -- daniel On 15/08/2019 16:27, Roger Riggs wrote: Looks good, Thanks, Roger On 8/15/19 11:22 AM, Claes Redestad wrote: (adding back core-libs-dev) Hi Roger, seems easy enough to add a writeReplace:

RFR: JDK-8152467: remove uses of anachronistic array declarations for method return type

2019-08-15 Thread Evgeny Mandrikov
Hello! Please review patch [1] for JDK-8152467 [2]. Also it needs a sponsor since I have only author status in OpenJDK Census [3]. After this change tier1 tests pass and I don't see other occurrences to replace in output of ag "\)\s*\[\s*\]" src With best regards, Evgeny Mandrikov [1]

Re: RFR: 8229773: Resolve permissions for code source URLs lazily

2019-08-15 Thread Claes Redestad
Hi Daniel, seems prudent, especially if we are to writeReplace the underlying collection on serialization. /Claes On 2019-08-15 17:10, Daniel Fuchs wrote: Hi Claes, I wonder if initialize() should check the state of the readOnly() flag - and if that's true, call perms.setReadOnly() ? see

Re: RFR: 8229773: Resolve permissions for code source URLs lazily

2019-08-15 Thread Roger Riggs
Looks good, Thanks, Roger On 8/15/19 11:22 AM, Claes Redestad wrote: (adding back core-libs-dev) Hi Roger, seems easy enough to add a writeReplace: http://cr.openjdk.java.net/~redestad/8229773/webrev.02 /Claes On 2019-08-15 16:54, Roger Riggs wrote: Hi Claes, I would recommend using

Re: RFR: 8229773: Resolve permissions for code source URLs lazily

2019-08-15 Thread Claes Redestad
(adding back core-libs-dev) Hi Roger, seems easy enough to add a writeReplace: http://cr.openjdk.java.net/~redestad/8229773/webrev.02 /Claes On 2019-08-15 16:54, Roger Riggs wrote: Hi Claes, I would recommend using writeReplace to serialize the PermissionCollection so the serialized form

Re: RFR: 8229773: Resolve permissions for code source URLs lazily

2019-08-15 Thread Daniel Fuchs
Hi Claes, I wonder if initialize() should check the state of the readOnly() flag - and if that's true, call perms.setReadOnly() ? see SecureClassLoader::getProtectionDomain(..) best regards, -- daniel On 15/08/2019 13:44, Claes Redestad wrote: Hi, On 2019-08-15 12:56, Alan Bateman wrote:

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 to

Re: RFR: 8229773: Resolve permissions for code source URLs lazily

2019-08-15 Thread Peter Firmstone
Hello Claes, The following code is included in the constructor of our SecurityManager implementation, I suspect we may need to add some classes to this list, perhaps this is something that needs documenting? Regards, Peter. /* The following ensures the classes we need are loaded early to

Re: RFR: 8229773: Resolve permissions for code source URLs lazily

2019-08-15 Thread Peter Firmstone
Hello Alan, This is related to URL and CodeSource and might be worth making a note of for future reference. Our software uses delayed dynamically assigned permissions via a policy provider, but for privileged domains that have AllPermission we make sure to assign this up front (We also

Re: RFR: JDK-8224594: Simplify jpackage Logging

2019-08-15 Thread Alexander Matveev
Looks good. Thanks, Alexander On 8/15/2019 1:50 PM, Andy Herrick wrote: ok - revised [3] to restore the second trace statement (now Log.verbose()) [3]: http://cr.openjdk.java.net/~herrick/8224594/webrev.02/ /Andy On 8/15/2019 3:40 PM, Andy Herrick wrote: The first of these doesn't convey

Re: RFR: JDK-8224594: Simplify jpackage Logging

2019-08-15 Thread Alexey Semenyuk
Looks good. - Alexey On 8/15/2019 4:50 PM, Andy Herrick wrote: ok - revised [3] to restore the second trace statement (now Log.verbose()) [3]: http://cr.openjdk.java.net/~herrick/8224594/webrev.02/ /Andy On 8/15/2019 3:40 PM, Andy Herrick wrote: The first of these doesn't convey any

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-8224594: Simplify jpackage Logging

2019-08-15 Thread Alexey Semenyuk
Sounds good. - Alexey On 8/15/2019 3:40 PM, Andy Herrick wrote: The first of these doesn't convey any additional information, since it is just parroting back the given option value. The second may be of use, and I could restore it as a Log.verbose(), but I felt the output here was

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

2019-08-15 Thread Aleksey Shipilev
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? -- Thanks, -Aleksey

Re: RFR: JDK-8224594: Simplify jpackage Logging

2019-08-15 Thread Alexey Semenyuk
Andy, What is the reason to remove log statements in http://cr.openjdk.java.net/~herrick/8224594/webrev.01/src/jdk.jpackage/share/classes/jdk/jpackage/internal/StandardBundlerParam.java.sdiff.html

Re: RFR: 8229773: Resolve permissions for code source URLs lazily

2019-08-15 Thread Alan Bateman
On 15/08/2019 16:22, Claes Redestad wrote: (adding back core-libs-dev) Hi Roger, seems easy enough to add a writeReplace: http://cr.openjdk.java.net/~redestad/8229773/webrev.02 This mostly looks good. In LazyCodeSourcePermissionCollection it think "initialize" should be renamed to

Re: RFR: JDK-8224594: Simplify jpackage Logging

2019-08-15 Thread Andy Herrick
ok - revised [3] to restore the second trace statement (now Log.verbose()) [3]: http://cr.openjdk.java.net/~herrick/8224594/webrev.02/ /Andy On 8/15/2019 3:40 PM, Andy Herrick wrote: The first of these doesn't convey any additional information, since it is just parroting back the given option

RFR: JDK-8224594: Simplify jpackage Logging

2019-08-15 Thread Andy Herrick
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). [1] https://bugs.openjdk.java.net/browse/JDK-8224594 [2] http://cr.openjdk.java.net/~herrick/8224594/ Thanks, Andy

Re: RFR: JDK-8224594: Simplify jpackage Logging

2019-08-15 Thread Andy Herrick
The first of these doesn't convey any additional information, since it is just parroting back the given option value. The second may be of use, and I could restore it as a Log.verbose(), but I felt the output here was incomplete, and would be better served by tracing in the caller, since this

Re: RFR: JDK-8224594: Simplify jpackage Logging

2019-08-15 Thread Alexander Matveev
Looks good and probably lets restore removed log statements. On 8/15/2019 12:09 PM, Alexey Semenyuk wrote: Andy, What is the reason to remove log statements in

Re: RFR: JDK-8152467: remove uses of anachronistic array declarations for method return type

2019-08-15 Thread Aleksey Shipilev
On 8/15/19 6:46 PM, Evgeny Mandrikov wrote: > [1] http://cr.openjdk.java.net/~godin/8152467/webrev.00/ This looks good. -- Thanks, -Aleksey

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

2019-08-15 Thread Aleksey Shipilev
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 to initialize bci to -1 for StackFrameInfo, >>

Re: flatMap still prevents short circuiting when using .iterator()

2019-08-15 Thread David Holmes
Re-directing to core-libs-dev David On 15/08/2019 7:48 pm, Stephen Buergler wrote: Not really sure where to send this. flatMap was fixed so that it doesn't prevent short circuiting. https://bugs.openjdk.java.net/browse/JDK-8075939 If you replace the test cases in the ticket with versions that