Re: Manifest copy constructor does not deeply copy individual section Attributes

[Philipp Kunz]

Hi Roger,
Thanks for looking into it and for your reply.I don't see a compulsory
reason to change the code.
In context of JDK-8217375 I happened to touch the issue and I agree to
Max to solve it differently in that particular case now by creating a
real deep copy based on reading the manifest bytes input stream again.h
ttps://mail.openjdk.java.net/pipermail/security-dev/2019-
January/019200.html
Even though I don't have a strong reason to change Manifest code any
more, in my opinion the current actual behavior is quite weird,
possibly too strange even to properly document without deprecating it
at the same time. Chances are that other people struggled over it just
like I did and will so again always getting the impression of a bug
whether documented or not. The current copy constructor already now
tries to deeply copy the original but clones only two out of three
collections. Hence, I'd guess the current code was developed with the
intention to create a deep copy back when it was developed. Maintaining
perfect backward compatibility would mean to also add a new deepClone
or similar method along with another constructor wouldn't it? The are
probably other uses but the only use of individual sections in
manifests that I'm currently aware of is jar signing and there it has
not been found so far as I'm aware of. While there certainly are risks
to clients there might be chances as well to fix issues in client code.
These are just some considerations in favor of changing the code but by
no means meant as a conclusion.
Philipp

On Tue, 2019-01-22 at 15:00 -0500, Roger Riggs wrote:
> Hi Phillip,
> 
> Did you discover need for a deeper copy?  If so, can you elaborate?
> It may be that adding a new constructor would be cleaner.
> 
> I'd suggest clarifying the documentation of existing methods as
> opposed
> to changing the spec and implementation and the possible risks
> to clients.
> 
> Thanks, Roger
> 
> On 01/21/2019 05:50 PM, Lance Andersen wrote:
> > Hi Philipp,
> > 
> > This is going to take some further discussion/input as this has
> > been documented for sometime as a shallow copy.
> > 
> > If there is a consensus  to move this forward, it will also require
> > a CSR.   I can help with that and sponsoring the updates,  if the
> > decision is to go ahead with the proposed change
> > 
> > Best
> > Lance
> > > On Jan 19, 2019, at 2:34 PM, Philipp Kunz  > > h> wrote:
> > > 
> > > Hi again,
> > > Just after having sent the previous mail I found Manifest::clone
> > > explicitly says it creates a shallow copy which is true only to a
> > > certain extent. It deeply copies main attributes as well as
> > > individual
> > > sections map already now and only shallowly copies individual
> > > sections
> > > attributes maps.
> > > I don't know about the background of it or why clone should only
> > > do a
> > > shallow copy but if clone should really create a shallow copy, it
> > > should probably create an even more shallow copy. In any case I
> > > figure
> > > some potential for clarification at least.
> > > Probably mostly because I already began a patch in the previous
> > > message, I continued and attached another patch for a deep copy
> > > approach. There might still be some reason not to deeply copy
> > > manifests
> > > which I don't intend to forestall.
> > > Philipp
> > > 
> > > On Sat, 2019-01-19 at 19:32 +0100, Philipp Kunz wrote:
> > > > Hi,
> > > > 
> > > > Creating a new manifest as a copy from an existing one using
> > > > the copy
> > > > constructor assigns the new manifest individual sections
> > > > entries map
> > > > the same values which are references to attributes as the
> > > > original
> > > > rather than copying them as well deeply resulting in two
> > > > manifests
> > > > pointing to the same attributes instances modifications to
> > > > which
> > > > always affect both manifests. See test and proposed fix in the
> > > > attached patch.
> > > > 
> > > > Regards,
> > > > Philipp
> > > 
> > > 
> > 
> >   
> >   
> > 
> >   Lance
> > Andersen| Principal Member of Technical Staff | +1.781.442.2037
> > Oracle Java Engineering
> > 1 Network Drive
> > Burlington, MA 01803
> > lance.ander...@oracle.com 
> > 
> > 
> > 

RE: RFR : 8217093: Support extended-length paths in parse_manifest.c on windows

[Baesken, Matthias]

Hello Alan,  here is a second webrev :

http://cr.openjdk.java.net/~mbaesken/webrevs/8217093.1/

I moved the  Windows-only code into a new file 

src/java.base/windows/native/libjli/parse_manifest_md.c


Best regards, Matthias


> -Original Message-
> From: Alan Bateman 
> Sent: Mittwoch, 16. Januar 2019 16:43
> To: Baesken, Matthias ; core-libs-
> d...@openjdk.java.net
> Subject: Re: RFR : 8217093: Support extended-length paths in
> parse_manifest.c on windows
> 
> On 16/01/2019 14:43, Baesken, Matthias wrote:
> > Hi Alan , do you think it is good to start  a UNIX   parse_manifest_md.c too
> with just a one-liner in it ?
> > Or do you prefer  only having the windows - file with the Windows coding
> and  keeping the UNIX one-linerfor   open_jarfile   in   parse_manifest.c 
> ?
> >
> I think this makes sense.
> 
> -Alan

Re: JDK-8214063: OpenJDK will not build on AIX while using the xlc 13.1 compiler

[Thomas Stüfe]

See also official downport documentation:

http://openjdk.java.net/projects/jdk-updates/approval.html

Cheers, Thomas

On Wed, Jan 23, 2019 at 12:46 PM Lindenmaier, Goetz <
goetz.lindenma...@sap.com> wrote:

> Hi Steve,
>
> feel free to request the downport of this change.
>
> For this, you must first check whether the change applies cleanly:
> cd jdk/jdk; hg export -r 7384e00d5860 > 8215962.changeset  (you find the
> hash in the JBS)
> cd jdk11u; hg import 8215962.changeset
>
> If so, you flag the bug in JBS as jdk11u-fix-request and add a
> comment to the bug why you want to downport this. The comment
> must start with "Fix Request".
> See for an example
> https://bugs.openjdk.java.net/browse/JDK-8215962
>
> You will get a message once the downport is approved.
> Ping me and I will push the change for you.
>
> If the change does not apply cleanly, you must send a RFR
> with the adapted webrev to the list where the change was
> originally reviewed.
> You must add a link to the RFR in pipermail to the comment
> in the bug.
> See for example:
> https://bugs.openjdk.java.net/browse/JDK-8213754
>
> Best regards,
>   Goetz.
>
>
>
> > -Original Message-
> > From: core-libs-dev  On Behalf
> Of
> > Steve Groeger
> > Sent: Mittwoch, 23. Januar 2019 12:18
> > To: OpenJDK Core Libs Developers 
> > Cc: Simonis, Volker ; Adam Farley8
> > 
> > Subject: JDK-8214063: OpenJDK will not build on AIX while using the xlc
> 13.1
> > compiler
> >
> > Hi Volker,
> >
> > Do you think it would be possible to get the change you committed to
> JDK12
> > http://cr.openjdk.java.net/~simonis/webrevs/2018/8214063/
> > for this issue https://bugs.openjdk.java.net/browse/JDK-8214063
> > back-ported to JDK11, as JDK11 is a LTS release and it woud be good to
> > have this fix?
> >
> > Also, we have received reports of other issues caused by the
> > -qvisibility=hidden
> > when using XLC 13.1 with JDK11 on AIX.
> >
> > One of these is related to :
> >
> > Desktop printer feature cannot detect by following exception.
> > Caused by: java.lang.UnsatisfiedLinkError:
> > sun/print/CUPSPrinter.initIDs()Z
> >
> > Thanks
> > Steve Groeger
> > IBM Runtime Technologies
> > Hursley, Winchester
> > Tel: (44) 1962 816911  Mobex: 279990  Mobile: 07718 517 129
> > Fax (44) 1962 816800
> > Lotus Notes: Steve Groeger/UK/IBM
> > Internet: groe...@uk.ibm.com
> >
> > Unless stated otherwise above:
> > IBM United Kingdom Limited - Registered in England and Wales with number
> > 741598.
> > Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6
> 3AU
> >
> >
> > > Hi Adam, Ichiroh,
> > >
> > > this is actually exactly what I wanted to propose in the mail I was
> > > just writing :)
> > >
> > > You may have noticed that I've renamed the bug to "8214063: [AIX]
> > > Disable symbol visibility flags". For the next step which will enable
> > > symbol visibility on AIX we already have the following issue "8204541:
> > > Correctly support AIX xlC 13.1 symbol visibility flags" [1]. Please
> > > assign yourself and use it once the issues are resolved.
> > >
> > > In the meantime I've tested your change and it works fine with xlC 12.
> > > I've only added a reference to the JBS id in the comments. Please find
> > > a webrev of the new version here:
> > >
> > > http://cr.openjdk.java.net/~simonis/webrevs/2018/8214063/
> > >
> > > I can sponsor that change once you and Ichiroh confirm that it also
> > > works for you with xlC 13 and I get one more review. I've opened a new
> > > thread for this on build-dev because the fix has now degenerated into
> > > a pure build change:
> > >
> > >
> > http://mail.openjdk.java.net/pipermail/build-dev/2018-
> > November/024096.html
> > >
> > > Please answer on that new thread.
> > >
> > > Regards,
> > > Volker
> > >
> > > PS: I don't think it makes too much sense investing into xlC 13.
> > > You're probably aware of "JEP 347: Adopt C++14 Language Features in
> > > HotSpot" [2] which will start introducing C++14 language features
> > > soon. So it may be wiser to test right with the new beta of xlC 16
> > > because, as far as I know, there's no chance to get C++124 support
> > > into xlC 13 anyway.
> > >
> > > [1] https://bugs.openjdk.java.net/browse/JDK-8204541
> > > [2] http://openjdk.java.net/jeps/347
> >
> >
> > Unless stated otherwise above:
> > IBM United Kingdom Limited - Registered in England and Wales with number
> > 741598.
> > Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6
> 3AU
>

Re: [RFR]: Per thread IO statistics in JFR

[Alan Bateman]

On 23/01/2019 15:59, Haug, Gunter wrote:

:

As Volker writes, we still think that the overhead of the up-calls is 
acceptable but it would be possible to introduce a switch which is based on a 
system property.

There are concerns with the approach and patch. The replies from David 
hint that he also has concerns. I think it would be useful to start out 
with a summary of what the app server is looking to do with these stats. 
I think we need to understand if collecting by thread is really the 
right way to do this as it may not be the right approach in the long 
term. The granularity is also important as you've instrumented a bunch 
of places that are surprising as they aren't regular file or socket I/O. 
I think it's important to understand what prototypes were done with 
instrumentation at the java level rather than in the native methods. 
This goes to the maintenance concern. There were suggestions of 
performance issues but it's not clear if those experiments were recent 
or from 10 years ago. For the libraries area then we really should be 
reducing the native code for maintenance and debugging reasons, maybe 
performance reasons too once we can start to make use of Panama. Finally 
I think it's important to see how this fits with the instrumentation 
that JFR and other potential instrumentation of the libraries going 
forward (native method tracking was mentioned). So overall I think there 
is a lot to discuss and write-up.


-Alan

Re: [RFR]: Per thread IO statistics in JFR

[Haug, Gunter]

Hi Alan, David

Here are the results of the measurements. As mentioned before, I had measured 
some of the standard Java benchmarks and there was no impact. This is probably 
not surprising to anybody as the overhead up-calls is minuscule in comparison 
to the calculations that take place and is far below the noise level.

A benchmark that is most prone to show the impact the up-calls would do fast 
(short) IO operations. The fastest operations I could think of were writing to 
/dev/null and reading from /dev/zero on Linux or macOS. I wrote benchmarks 
which write/read just one byte one million times in a loop using the jmh 
framework (https://openjdk.java.net/projects/code-tools/jmh/). 

The results measured on my mac were all over the place and I moved to a 
otherwise empty Linux host. Results were more stable there. jmh runs the 
benchmark multiple times for each invocation and calculates an error within one 
invocation. It turned out that invocations which had a large error showed 
poorer performance, obviously because something happened on the host in 
parallel what degraded performance. I invoked the benchmarks multiple times and 
decided to discard measurements with an error of more than 0.01. This value is 
rather random but it was achieved by about one out of four invocations and I 
think we can get rid of the outliers.

It turned out that even with this setup the impact of the up-call is below 
noise level. In fact, the write benchmark was faster with the up-calls what is 
not plausible, of course. The read benchmark showed the best and the poorest 
performance without the up-calls while the results for a JDK with the up-calls 
(see below for the actual results).

As Volker writes, we still think that the overhead of the up-calls is 
acceptable but it would be possible to introduce a switch which is based on a 
system property.

@Thomas, thanks for your input! I think all your suggestions are well funded. 
I'll reply in more detail if/when we can alleviate the more general concerns.

Best regards,
Gunter

Write without up-call
Mode  Cnt  Score   Error  Units
thrpt   25  1.867   0.005  ops/s
thrpt   25  1.866   0.006  ops/s
thrpt   25  1.865   0.004  ops/s
thrpt   25  1.860   0.010  ops/s

Write with up-call
Mode  Cnt  Score   Error  Units
thrpt   25  1.880   0.010  ops/s
thrpt   25  1.884   0.007  ops/s
thrpt   25  1.887   0.004  ops/s
thrpt   25  1.879   0.010  ops/s


Read without up-calls
Mode  Cnt  Score   Error  Units
thrpt   25  2.145   0.008  ops/s
thrpt   25  2.145   0.010  ops/s
thrpt   25  2.129   0.010  ops/s
thrpt   25  2.122   0.009  ops/s

Read with up-calls
Mode  Cnt  Score   Error  Units
thrpt   25  2.136   0.007  ops/s
thrpt   25  2.132   0.003  ops/s
thrpt   25  2.130   0.006  ops/s
thrpt   25  2.129   0.009  ops/s









On 17.01.19, 18:31, "Volker Simonis"  wrote:

On Thu, Jan 17, 2019 at 1:31 PM Alan Bateman  
wrote:
>
> On 17/01/2019 07:23, Thomas Stüfe wrote:
> > :
> >
> > Do you object against keeping these counters (which basically boils
> > down to Thread::current->stat_structure->counter++)? Or do you even
> > object against making upcalls into the jvm? Note that, if deemed
> > necessary, we could omit updating the counters unless JFR or our
> > extended thread dumps are activated (which are the consumers of the
> > counters).
> >
> > In any case, I would have assumed the costs for upcall + counter
> > update to be insignificant compared to the IO calls. We should of
> > course measure that.
> >
> > If you generally object upcalls into the libjvm for
> > statistical/monitoring reasons, this would make matters on a number of
> > fronts more complicated. For instance, it was discussed extending NMT
> > coverage to the JDK - which is already in part reality at
> > Unsafe.AllocateMemory - and this would have to be done with upcalls too.
> >
> There are many issues here that will need write-up and discussion, maybe
> a JEP if discussions converge on a proposal to bring into the main line
> as this is a significant change with implications for many areas of the
> platform.

We could certainly create a new JEP for this feature, but in the end
it is rather trivial. In fact it can be expressed in a single
sentence: instrument all native IO functions to collect the number of
read and written bytes. As stated before, the overhead is minimal
(Gunter will provide some concrete benchmarks soon) and could even be
dropped to ~zero if you really insist on this (e.g. by wrapping all
calls by a switch which is based on a system property). But honestly
speaking we don't think this is really necessary.

I want to stress the fact that this is not a wacky idea Gunter came up
with but a feature which we're using for almost 10 years in our SAP
JVM. It is running in all our enterprise scenarios without ever having
caused any problems.

Re: RFR: JDK-8216558: Lookup.unreflectSetter(Field) fails to throw IllegalAccessException for final fields

[Adam Farley8]

Hi Mandy,

Food for thought:

- From John's email, it looked like he was advocating a clarification 
change to the spec (which requires no CSR).

- A test case is attached to the bug. It displays "current behaviour", but 
doesn't show a clear "pass" or "fail", because at the start of this there 
appeared to be disagreement over what the correct behaviour was.

- Good idea on the new line. I'll add it to the webrev shortly.

- As for the comments, I think I see what John was going for, but I don't 
understand his first change. This seems to be the opposite of what we're 
trying for here.

+ * In the case of a field setter function on a {@code final} field,
+ * finality enforcement is treated as a kind of access control,
+ * and the lookup will fail, except in special cases of
+ * {@link Lookup#unreflectSetter Lookup.unreflectSetter}.

Perhaps he means it will still fail, but for the reasons we've discussed 
rather than anything connected to access control?

Best Regards

Adam Farley 
IBM Runtimes


Mandy Chung  wrote on 16/01/2019 23:52:03:

> From: Mandy Chung 
> To: Adam Farley8 
> Cc: core-libs-dev 
> Date: 16/01/2019 23:54
> Subject: Re: RFR: JDK-8216558: Lookup.unreflectSetter(Field) fails 
> to throw IllegalAccessException for final fields
> 
> Hi Adam,

> On 1/14/19 9:10 AM, Adam Farley8 wrote:
> As for the CSR process, I'm unfamiliar with it. I've modified the 
> comment for the unreflectSetter method in the webrev. Do let me know if 
> more needs to be done.
> 
> The CSR process is straight-forward.  CSR FAQ [1] may help you get 
started. 
> 
> As for the fix, John has suggested the spec wording for this issue. 
> It's fine with the unreflectField method to have the explicit check 
> and throw field.makeAccessException(...).   The exception message 
> could match the proposed spec wording like:
>  if(isSetter && field.isStatic() && field.isFinal())
>  throw field.makeAccessException("static final field has no 
> write access", this);
> 
> You have a very long line and the throw statement can start in the 
> next line.  
> 
> In addition, please include a test case for this change.
> 
> Thanks
> Mandy
> [1] https://wiki.openjdk.java.net/display/csr/CSR+FAQs 
Unless stated otherwise above:
IBM United Kingdom Limited - Registered in England and Wales with number 
741598. 
Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU

Re: RFR 8217339: ClassCircularityError loading NumberFormatProvider

[Naoto Sato]

+1

Naoto

On 1/22/19 3:25 PM, Mandy Chung wrote:



On 1/22/19 2:25 PM, Roger Riggs wrote:

Hi Mandy,

Updated webrev:
http://cr.openjdk.java.net/~rriggs/webrev-circ-error-8217339-2/




Looks good.



Other changes look good.

BTW,

> I have not found a reproducer for jdk 12, it only occurs on new 
work for

> jdk 13.

Is this new code in 13? l10n causes this type of circularity 
exception during the VM boot up.




I also want to understand what changes in 13 causes the difference.

I was prototyping some more properties ideas.
I suspect that in the previous init sequence, the Locale had been cached.
A closed test had failed, in part because it had changed the default 
locale

and then installed a security manager.


I see how ConstantDesc gets initialized during initialization of 
VarHandle from
your stack trace.  I can't reproduce it and so I assume it was triggered 
by your
local change.  In any case, converting String.format to string concat is 
fine.


I am sure Claes may look at the startup and class initialization if we 
can avoid

loading VarHandleDesc from clinit.

Mandy

Re: RFR: JDK-8217393 Re: Clarification in Attributes equal

[Roger Riggs]

+1 to retaining and correcting the javadoc to match the implementation.

Thanks, Roger

On 01/22/2019 07:47 PM, Lance Andersen wrote:

On Jan 22, 2019, at 12:02 PM, Alan Bateman  wrote:

On 19/01/2019 12:46, Lance Andersen wrote:

Hi all,

Please review the  fix for JDK-8217393 which updates the javadocs for 
Attriibutes::equals to clarify its behavior to match its implementation

—
hg diff
diff -r c5d6b4480c6c src/java.base/share/classes/java/util/jar/Attributes.java
--- a/src/java.base/share/classes/java/util/jar/Attributes.java Thu Jan 17 
13:46:12 2019 -0800
+++ b/src/java.base/share/classes/java/util/jar/Attributes.java Sat Jan 19 
07:35:55 2019 -0500
@@ -265,9 +265,10 @@
  }
/**
- * Compares the specified Attributes object with this Map for equality.
- * Returns true if the given object is also an instance of Attributes
- * and the two Attributes objects represent the same mappings.
+ * Compares the specified object with this Map for equality.
+ * Returns true if the given object is also a Map
+ * and the two objects represent the same Manifest
+ * attribute name-value mappings.


I think this looks okay although I like Martin's suggestion to just inherit the 
javadoc as Attributes is a Map.

I had thought about that but felt that keeping the javadoc similar to what it 
has been might be the better approach given it has been around since JDK 1.2

If we were to inherit the javadoc, we should probably look at the rest of the 
methods to see where else it would  make sense to inherit the javadoc

Best
Lance

-Alan

  
   

  Lance Andersen| 
Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering
1 Network Drive
Burlington, MA 01803
lance.ander...@oracle.com 

Re: RFR: JDK-8216558: Lookup.unreflectSetter(Field) fails to throw IllegalAccessException for final fields

[Mandy Chung]

Hi Adam,


On 1/23/19 8:33 AM, Adam Farley8 wrote:

Hi Mandy,

Food for thought:

- From John's email, it looked like he was advocating a clarification 
change to the spec (which requires no CSR).




While the new behavior was intended when the spec was written, this fix 
changes the current behavior and a CSR will be needed to document the 
incompatible change.  The current behavior of unreflectSetter may return 
a MethodHandle of a static final Field object with accessible flag set 
and then invoke MH to change the static final field value.  The new 
behavior of unreflectSetter throws IAE as static final FIeld does not 
have write access.


- * @throws IllegalAccessException if access checking fails
+ * @throws IllegalAccessException if access checking fails,
+ * or if the field is {@code final} and write access
+ * is not enabled on the {@code Field} object

- A test case is attached to the bug. It displays "current behaviour", 
but doesn't show a clear "pass" or "fail", because at the start of 
this there appeared to be disagreement over what the correct behaviour 
was.


- Good idea on the new line. I'll add it to the webrev shortly.

- As for the comments, I think I see what John was going for, but I 
don't understand his first change. This seems to be the opposite of 
what we're trying for here.


+     * In the case of a field setter function on a {@code final} field,
+     * finality enforcement is treated as a kind of access control,
+     * and the lookup will fail, except in special cases of
+     * {@link Lookup#unreflectSetter Lookup.unreflectSetter}.

Perhaps he means it will still fail, but for the reasons we've 
discussed rather than anything connected to access control?




http://cr.openjdk.java.net/~mchung/jdk13/webrevs/8216558/webrev.01/

This webrev showing @throws change should show the spec clarification on 
findSetter and findStaticSetter that throws IAE when looking up a setter 
on a final field and the spec of unreflectSetter states that it will 
return a method handle only if the corresponding call to Field::set 
returns normally.    This webrev is not a review request but rather 
showing the fix we are discussing here.


Hope this clarifies it.

Mandy

JDK-6982173: Small problem causing thousands of wasted CPU hours

[Jan Peter Stotz]

Hi,

like many other I ran into bug JDK-698217 which is about 
AbstractSet.removeAll() and it's "aptitude" in wasting CPU time if you 
accidentally hit this bug. And there are hundred of developers hitting 
this bug every year.


I simply don't understand why there was no progress in 8 years, on a 
severe performance issue (a removeAll method on an efficient set that 
can require O(n^2)!) caused by code that was written to speed-up the 
removeAll implementation.


Which makes this bug worse is that it is triggered by the relative size 
of the current set compared to the collection passed as parameter.
Therefore for most developers this means not to use this buggy function 
at all (once they realized how worse it is).


IMHO there is a extreme simple workaround for avoiding this problem. The 
relevant part of the current implementation is as follows:


if (size() > c.size()) {
for (Object e : c)
modified |= remove(e);
} else {
for (Iterator i = iterator(); i.hasNext(); ) {
if (c.contains(i.next())) {
i.remove();
modified = true;
}
}
}

The problem is that the code in the first if block only makes sense if 
the collections passed as parameter implements the remove(Object o) 
method in an efficient way - this means with a complexity of less than 
O(n).
Java currently has no way to identify Collection implementations that 
satisfy this property we should at least implement a workaround that 
prevents the Java developers form unexpected "damage".
Which mean that the first if block should only be triggered if we are 
sure that it is faster than the second case (because removing an element 
from a Set is usually implemented efficiently).


Therefore I would propose a patch that no only compare the size of the 
current set and the given collection but also makes sure that the given 
collection is a Set or a Map:


if (size() > c.size() && (c instanceof Set || c instanceof Map) {
 for (Object e : c)
 modified |= remove(e);
} else {
  

Jan

[13] RFR: 8217366: ZoneStrings are not populated for all the Locales

[Naoto Sato]

Hi,

Please review the fix to the following issue:

https://bugs.openjdk.java.net/browse/JDK-8217366

The proposed changeset is located at:

http://cr.openjdk.java.net/~naoto/8217366/webrev.00/

The gist of the issue is that CLDR's "no inheritance marker" slipped 
into the final zone strings array, which should be converted to GMT format.


Naoto

Re: [13] RFR: 8217366: ZoneStrings are not populated for all the Locales

[Roger Riggs]

Hi Naoto,

Looks fine to me.

Roger


On 01/23/2019 12:47 PM, Naoto Sato wrote:

Hi,

Please review the fix to the following issue:

https://bugs.openjdk.java.net/browse/JDK-8217366

The proposed changeset is located at:

http://cr.openjdk.java.net/~naoto/8217366/webrev.00/

The gist of the issue is that CLDR's "no inheritance marker" slipped 
into the final zone strings array, which should be converted to GMT 
format.


Naoto

Re: RFR: 8214533 IBM-29626C is required for AIX default charset

[Roger Riggs]

Hi Ichiroh,

Sorry for the delay,

yes, the change looks fine.

I'll sponsor, and push it tomorrow.

Thanks, Roger


On 01/23/2019 07:21 AM, Ichiroh Takiguchi wrote:

Hello.

Could you review the fix and give your suggestion ?
Thanks,
Ichiroh Takiguchi

On 2019-01-16 21:32, Ichiroh Takiguchi wrote:

Hello Alan and Roger.

I appreciate your suggestions.

Could you review the fix again ?

Bug:    https://bugs.openjdk.java.net/browse/JDK-8214533
Change: https://cr.openjdk.java.net/~itakiguchi/8214533/webrev.02/

I used "hg cp" command.
If I should use git format, please let me know.

I changed Copyright year on IBM29626C.java.template and charsets file.

Thanks,
Ichiroh Takiguchi

On 2019-01-16 01:01, Roger Riggs wrote:

Hi,

I'd suggest using 2002, 2019, for the copyright, since much of the
code in the new
file comes from an older source.

$.02, Roger


On 01/15/2019 10:43 AM, Alan Bateman wrote:

On 15/01/2019 00:51, Ichiroh Takiguchi wrote:

Hello Alan.

Could you review the fix again ?

Bug:    https://bugs.openjdk.java.net/browse/JDK-8214533
Change: https://cr.openjdk.java.net/~itakiguchi/8214533/webrev.01/

I added IBM29626C charset as standard way.
Please give any suggestion and question.
This looks okay, assuming it's `hg copy` of EUR_JP.template to 
create IBM29626C.java.template rather than a `hg mv` (the webrev 
makes it look like a move/rename).


-Alan

Re: JDK-6982173: Small problem causing thousands of wasted CPU hours

[Federico Peralta Schaffner]

Hi Jan,

I'm amazed to see this performance problem has persisted for so long. As a
Java developer, I wasn't even aware of it, so thanks for the information.

>
> The relevant part of the current implementation is as follows:
>
>  if (size() > c.size()) {
>  for (Object e : c)
>  modified |= remove(e);
>  } else {
>  for (Iterator i = iterator(); i.hasNext(); ) {
>  if (c.contains(i.next())) {
>  i.remove();
>  modified = true;
>  }
>  }
>  }
>
> The problem is that the code in the first if block only makes sense if
> the collections passed as parameter implements the remove(Object o)
> method in an efficient way - this means with a complexity of less than
> O(n).

Now, how have you come to the conclusion that the problem is in the first
branch of the above if statement, and that it is related to the passed
collection's implementation details of the remove(Object o) method? I think
this is not the problem, as elements are removed from this set and not from
the passed collection.

In fact, the problem is in the else branch (when this set is smaller than
the passed collection). The problem is in the following line:

  if (c.contains(i.next())) {

That c.contains(...) might not be implemented in an optimized way, i.e. c
might be an ArrayList, which uses a linear search to check if the element
belongs to it.

> Java currently has no way to identify Collection implementations that
> satisfy this property (...)

Yes, you are correct. While there exists the RandomAccess marker interface
for List implementations, there is no analogous marker interface for
neither Set nor Map implementations that use hashing, i.e. something that
could have been named "HashAccess".

So I wonder and if the members of this group find it plausible to add such
HashAccess marker interface to HashSet and HashMap (and to any other
implementations that use hashing). If (as I suspect) introducing further
marker interfaces is not desirable (or has been tacitly forbidden), then
what alternatives are available and would be a good fit here?

For the record, if the HashAccess marker interface had ever existed, the
implementation could have been changed to:

  if (size() > c.size()) {
  for (Object e : c)
  modified |= remove(e);
  } else {
  Collection cc = c instanceof HashAccess ? c : new
HashSet<>(c);
  for (Iterator i = iterator(); i.hasNext(); ) {
  if (cc.contains(i.next())) {
  i.remove();
  modified = true;
  }
  }
  }

This way, the contains invocation in `if (cc.contains(i.next())) {` would
have been optimized.

Thanks,
fps.-

Re: High memory usage / leaks was: Best mailing list for JVM embedding

[Sean Mullan]

On 1/22/19 8:50 PM, Bernd Eckenfels wrote:

I don’t think the launcher is doing this, it is the class loader, that’s 
nothing new. You can turn on verbose security debug to see it in all versions.


Yes, and it only verifies the signature(s) on the JAR. It doesn't 
validate the certificate chain.


--Sean



--
https://Bernd.eckenfels.net


Von: core-libs-dev  im Auftrag von Robert 
Marcano 
Gesendet: Mittwoch, Januar 23, 2019 2:18 AM
An: Alan Bateman
Cc: OpenJDK Dev list; core-libs-dev Libs
Betreff: Re: High memory usage / leaks was: Best mailing list for JVM embedding

On Tue, Jan 22, 2019, 5:53 AM Alan Bateman 
On 22/01/2019 4:48 am, Robert Marcano wrote:

:

So the question now is, why signed jars could affect the memory usage
of an application (we aren't doing JAR verification on our custom
launcher, yet), just by being on the java.class.path? IIRC the
initial application classpath JARs were never verified previously (by
the java launcher alone, without JNLP around).

Note: Tested with JARs signed with a self signed certificate and with
one signed with a private CA. At most, signing the JARs could slow
down the start up if it is now expected to these being verified by
the java launcher (is it true?) but not higher memory usage and no
reductions after a GC cycle but constants heap size increases.

Signed JARs can be expensive to verify, esp. on first usage as the
verification is likely to result in early loading of a lot of security
classes and infrastructure. If you can narrow down the apparently memory
leak to a small test case with analysis to suggest it's a JDK bug then
it would be good to get a bug submitted.

-Alan



Greeting. Sure, I will work on a distributable reproduction of the problem
today but it is new to me that the java launcher do JARs verification now.
If it is doing it I doesn't make sense to me, because a self signed or
unrecognized CA doesn't trigger a validation error.

JDK-8214063: OpenJDK will not build on AIX while using the xlc 13.1 compiler

[Steve Groeger]

Hi Volker, 

Do you think it would be possible to get the change you committed to JDK12
http://cr.openjdk.java.net/~simonis/webrevs/2018/8214063/
for this issue https://bugs.openjdk.java.net/browse/JDK-8214063 
back-ported to JDK11, as JDK11 is a LTS release and it woud be good to 
have this fix?

Also, we have received reports of other issues caused by the 
-qvisibility=hidden
when using XLC 13.1 with JDK11 on AIX. 

One of these is related to :

Desktop printer feature cannot detect by following exception.
Caused by: java.lang.UnsatisfiedLinkError: 
sun/print/CUPSPrinter.initIDs()Z

Thanks
Steve Groeger
IBM Runtime Technologies
Hursley, Winchester
Tel: (44) 1962 816911  Mobex: 279990  Mobile: 07718 517 129
Fax (44) 1962 816800
Lotus Notes: Steve Groeger/UK/IBM
Internet: groe...@uk.ibm.com

Unless stated otherwise above:
IBM United Kingdom Limited - Registered in England and Wales with number 
741598.
Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU


> Hi Adam, Ichiroh,
> 
> this is actually exactly what I wanted to propose in the mail I was
> just writing :)
> 
> You may have noticed that I've renamed the bug to "8214063: [AIX]
> Disable symbol visibility flags". For the next step which will enable
> symbol visibility on AIX we already have the following issue "8204541:
> Correctly support AIX xlC 13.1 symbol visibility flags" [1]. Please
> assign yourself and use it once the issues are resolved.
> 
> In the meantime I've tested your change and it works fine with xlC 12.
> I've only added a reference to the JBS id in the comments. Please find
> a webrev of the new version here:
> 
> http://cr.openjdk.java.net/~simonis/webrevs/2018/8214063/
> 
> I can sponsor that change once you and Ichiroh confirm that it also
> works for you with xlC 13 and I get one more review. I've opened a new
> thread for this on build-dev because the fix has now degenerated into
> a pure build change:
> 
> 
http://mail.openjdk.java.net/pipermail/build-dev/2018-November/024096.html
> 
> Please answer on that new thread.
> 
> Regards,
> Volker
> 
> PS: I don't think it makes too much sense investing into xlC 13.
> You're probably aware of "JEP 347: Adopt C++14 Language Features in
> HotSpot" [2] which will start introducing C++14 language features
> soon. So it may be wiser to test right with the new beta of xlC 16
> because, as far as I know, there's no chance to get C++124 support
> into xlC 13 anyway.
> 
> [1] https://bugs.openjdk.java.net/browse/JDK-8204541
> [2] http://openjdk.java.net/jeps/347


Unless stated otherwise above:
IBM United Kingdom Limited - Registered in England and Wales with number 
741598. 
Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU

RE: JDK-8214063: OpenJDK will not build on AIX while using the xlc 13.1 compiler

[Lindenmaier, Goetz]

Hi Steve,

feel free to request the downport of this change.

For this, you must first check whether the change applies cleanly:
cd jdk/jdk; hg export -r 7384e00d5860 > 8215962.changeset  (you find the hash 
in the JBS)
cd jdk11u; hg import 8215962.changeset

If so, you flag the bug in JBS as jdk11u-fix-request and add a 
comment to the bug why you want to downport this. The comment
must start with "Fix Request".
See for an example
https://bugs.openjdk.java.net/browse/JDK-8215962

You will get a message once the downport is approved.
Ping me and I will push the change for you.

If the change does not apply cleanly, you must send a RFR
with the adapted webrev to the list where the change was 
originally reviewed.
You must add a link to the RFR in pipermail to the comment
in the bug.
See for example:
https://bugs.openjdk.java.net/browse/JDK-8213754

Best regards,
  Goetz.



> -Original Message-
> From: core-libs-dev  On Behalf Of
> Steve Groeger
> Sent: Mittwoch, 23. Januar 2019 12:18
> To: OpenJDK Core Libs Developers 
> Cc: Simonis, Volker ; Adam Farley8
> 
> Subject: JDK-8214063: OpenJDK will not build on AIX while using the xlc 13.1
> compiler
> 
> Hi Volker,
> 
> Do you think it would be possible to get the change you committed to JDK12
> http://cr.openjdk.java.net/~simonis/webrevs/2018/8214063/
> for this issue https://bugs.openjdk.java.net/browse/JDK-8214063
> back-ported to JDK11, as JDK11 is a LTS release and it woud be good to
> have this fix?
> 
> Also, we have received reports of other issues caused by the
> -qvisibility=hidden
> when using XLC 13.1 with JDK11 on AIX.
> 
> One of these is related to :
> 
> Desktop printer feature cannot detect by following exception.
> Caused by: java.lang.UnsatisfiedLinkError:
> sun/print/CUPSPrinter.initIDs()Z
> 
> Thanks
> Steve Groeger
> IBM Runtime Technologies
> Hursley, Winchester
> Tel: (44) 1962 816911  Mobex: 279990  Mobile: 07718 517 129
> Fax (44) 1962 816800
> Lotus Notes: Steve Groeger/UK/IBM
> Internet: groe...@uk.ibm.com
> 
> Unless stated otherwise above:
> IBM United Kingdom Limited - Registered in England and Wales with number
> 741598.
> Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU
> 
> 
> > Hi Adam, Ichiroh,
> >
> > this is actually exactly what I wanted to propose in the mail I was
> > just writing :)
> >
> > You may have noticed that I've renamed the bug to "8214063: [AIX]
> > Disable symbol visibility flags". For the next step which will enable
> > symbol visibility on AIX we already have the following issue "8204541:
> > Correctly support AIX xlC 13.1 symbol visibility flags" [1]. Please
> > assign yourself and use it once the issues are resolved.
> >
> > In the meantime I've tested your change and it works fine with xlC 12.
> > I've only added a reference to the JBS id in the comments. Please find
> > a webrev of the new version here:
> >
> > http://cr.openjdk.java.net/~simonis/webrevs/2018/8214063/
> >
> > I can sponsor that change once you and Ichiroh confirm that it also
> > works for you with xlC 13 and I get one more review. I've opened a new
> > thread for this on build-dev because the fix has now degenerated into
> > a pure build change:
> >
> >
> http://mail.openjdk.java.net/pipermail/build-dev/2018-
> November/024096.html
> >
> > Please answer on that new thread.
> >
> > Regards,
> > Volker
> >
> > PS: I don't think it makes too much sense investing into xlC 13.
> > You're probably aware of "JEP 347: Adopt C++14 Language Features in
> > HotSpot" [2] which will start introducing C++14 language features
> > soon. So it may be wiser to test right with the new beta of xlC 16
> > because, as far as I know, there's no chance to get C++124 support
> > into xlC 13 anyway.
> >
> > [1] https://bugs.openjdk.java.net/browse/JDK-8204541
> > [2] http://openjdk.java.net/jeps/347
> 
> 
> Unless stated otherwise above:
> IBM United Kingdom Limited - Registered in England and Wales with number
> 741598.
> Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU

Re: RFR: 8214533 IBM-29626C is required for AIX default charset

[Ichiroh Takiguchi]

Hello.

Could you review the fix and give your suggestion ?
Thanks,
Ichiroh Takiguchi

On 2019-01-16 21:32, Ichiroh Takiguchi wrote:

Hello Alan and Roger.

I appreciate your suggestions.

Could you review the fix again ?

Bug:https://bugs.openjdk.java.net/browse/JDK-8214533
Change: https://cr.openjdk.java.net/~itakiguchi/8214533/webrev.02/

I used "hg cp" command.
If I should use git format, please let me know.

I changed Copyright year on IBM29626C.java.template and charsets file.

Thanks,
Ichiroh Takiguchi

On 2019-01-16 01:01, Roger Riggs wrote:

Hi,

I'd suggest using 2002, 2019, for the copyright, since much of the
code in the new
file comes from an older source.

$.02, Roger


On 01/15/2019 10:43 AM, Alan Bateman wrote:

On 15/01/2019 00:51, Ichiroh Takiguchi wrote:

Hello Alan.

Could you review the fix again ?

Bug:    https://bugs.openjdk.java.net/browse/JDK-8214533
Change: https://cr.openjdk.java.net/~itakiguchi/8214533/webrev.01/

I added IBM29626C charset as standard way.
Please give any suggestion and question.
This looks okay, assuming it's `hg copy` of EUR_JP.template to create 
IBM29626C.java.template rather than a `hg mv` (the webrev makes it 
look like a move/rename).


-Alan

Multi-release jar file - Should the "Multi-Release" mainfest attribute have a specific value?

[Jaikiran Pai]

Hi,

The Multi-Release JarFile support JEP[1] notes that the jar file is
expected to contain a main attribute:

Multi-Release: true

The JarFile javadoc[1] on the other hand makes no mention of the value
for such an attribute and just mentions that the Multi-Release main
attribute needs to be present:

"A multi-release jar file is a jar file that contains a manifest with a
main attribute named "Multi-Release",..."

Which one of these 2 docs is expected to be the formal specification?
I'm guessing it's the javadoc, in which case, is it intentional that the
value for this attribute isn't mentioned or is it just a miss? My quick
tests show that the implementation of this feature does indeed expect
the value to be a "true" for it to consider the versioned entries in a
multi-release jar.

[1] http://openjdk.java.net/jeps/238

[2]
https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/util/jar/JarFile.html

-Jaikiran

Re: Multi-release jar file - Should the "Multi-Release" mainfest attribute have a specific value?

[Alan Bateman]

On 23/01/2019 14:27, Jaikiran Pai wrote:

Hi,

The Multi-Release JarFile support JEP[1] notes that the jar file is
expected to contain a main attribute:

Multi-Release: true

The JarFile javadoc[1] on the other hand makes no mention of the value
for such an attribute and just mentions that the Multi-Release main
attribute needs to be present:

"A multi-release jar file is a jar file that contains a manifest with a
main attribute named "Multi-Release",..."

Which one of these 2 docs is expected to be the formal specification?
I'm guessing it's the javadoc, in which case, is it intentional that the
value for this attribute isn't mentioned or is it just a miss? My quick
tests show that the implementation of this feature does indeed expect
the value to be a "true" for it to consider the versioned entries in a
multi-release jar.

The JAR file spec [1] is the specification. There's a lengthy section 
defining MR JARs. The API docs are also specification, for the APIs that 
provide access to JAR files, including MR JARs. So yes, you need the 
"Multi-Release" attribute in the main section of the JAR file manifest 
and it needs to have a value of "true" to enable. There is also a 
JDK-specific property, documented in the JarFile docs, to override the 
configuration where needed.


-Alan

[1] https://docs.oracle.com/en/java/javase/11/docs/specs/jar/jar.html

Re: Multi-release jar file - Should the "Multi-Release" mainfest attribute have a specific value?

[Jaikiran Pai]

Thank you Alan.

-Jaikiran

On 23/01/19 8:09 PM, Alan Bateman wrote:
> On 23/01/2019 14:27, Jaikiran Pai wrote:
>> Hi,
>>
>> The Multi-Release JarFile support JEP[1] notes that the jar file is
>> expected to contain a main attribute:
>>
>> Multi-Release: true
>>
>> The JarFile javadoc[1] on the other hand makes no mention of the value
>> for such an attribute and just mentions that the Multi-Release main
>> attribute needs to be present:
>>
>> "A multi-release jar file is a jar file that contains a manifest with a
>> main attribute named "Multi-Release",..."
>>
>> Which one of these 2 docs is expected to be the formal specification?
>> I'm guessing it's the javadoc, in which case, is it intentional that the
>> value for this attribute isn't mentioned or is it just a miss? My quick
>> tests show that the implementation of this feature does indeed expect
>> the value to be a "true" for it to consider the versioned entries in a
>> multi-release jar.
>>
> The JAR file spec [1] is the specification. There's a lengthy section
> defining MR JARs. The API docs are also specification, for the APIs
> that provide access to JAR files, including MR JARs. So yes, you need
> the "Multi-Release" attribute in the main section of the JAR file
> manifest and it needs to have a value of "true" to enable. There is
> also a JDK-specific property, documented in the JarFile docs, to
> override the configuration where needed.
>
> -Alan
>
> [1] https://docs.oracle.com/en/java/javase/11/docs/specs/jar/jar.html