RFR[JDK10]: 8195837: (tz) Support tzdata2018c

[Ramanand Patil]

Hi all,
Please review the latest TZDATA integration (tzdata2018c) into JDK10.
Bug: https://bugs.openjdk.java.net/browse/JDK-8195837
Webrev: http://cr.openjdk.java.net/~rpatil/8195837/webrev.00/

All the TimeZone related tests are passed after integration.

Regards,
Ramanand.

RFR: 8196331: Optimize Character.digit for latin1 input

[Claes Redestad]

Hi,

for the latin1 block of CharacterData we can improve the
digit(int, int) implementation by providing an optimized lookup
table. This improves microbenchmarks exercising Integer.parseInt,
Long.parseLong and UUID.fromString etc by around 50%for typical
inputs.

Webrev: http://cr.openjdk.java.net/~redestad/8196331/open.00/
Bug: https://bugs.openjdk.java.net/browse/JDK-8196331

The lookup array is pre-calculated to minimize startup impact
(adds 1,027 bytecodes executed during initialization)

/Claes

Re: Optimizing UUID#fromString(String)

[Claes Redestad]

Hi,

I've filed two RFEs:

- https://bugs.openjdk.java.net/browse/JDK-8196331 for providing a
  fast-path for ASCII/Latin1 input to Character.digit. Patch out
  for review
- https://bugs.openjdk.java.net/browse/JDK-8196334 for tracking
  the optimizations specific to UUID proposed here

Jon, if you want to provide a patch I'd be happy to sponsor. I have
a few ideas on how to retrofit existing behavior that we can discuss
offline. Also, if you haven't filed an OCA then this might be a good
time to do so[1]

Thanks!

/Claes

[1] http://openjdk.java.net/contribute/

On 2018-01-29 00:19, Claes Redestad wrote:

Hi Jon,

On 2018-01-27 17:05, Jon Chambers wrote:

Regardless, I wanted to call this optimization opportunity to your
attention, and would be happy to offer a proper patch if this seems 
like a

worthwhile change.


this does looks promising - and at least points out there might be
some performance issues lurking here!

Found another compatibility issue, however: The current
UUID.fromString allow use of any unicode digits parseable into
digits. Say, Arabic digits (U+0660 - U+0669):

jshell> UUID.fromString("\u0669-\u0669-\u0669-\u0669-\u0669");
$2 ==> 0009-0009-0009-0009-0009

That you're getting some improvement from avoiding
Character.digit(..) stands out in a way, so I wondered if there's
something going on here... and it does seem that this method could
be optimized by providing a fast-path in the CharacterDataLatin1
plane...

I experimented with a few solutions, and the best I could find was
a 256 element byte[] lookup table for digits in CharacterDataLatin1
(spanning the entire 8-bit space of CharacterDataLatin1 allows us
to avoid a branch):

http://cr.openjdk.java.net/~redestad/scratch/char.digit.00/

This speeds up your micro testing java.util.UUID
(UUIDBenchmark.benchmarkUUIDFromString) by ~1.6x. The
improvement is there on all counters when looking at -prof perfnorm
(less loads, stores, branches, instructions, better CPI etc)[1], so seems
like a pretty straightforward thing to consider independently.

Your FastUUID implementation is still quite a bit faster, though, but
it'd be good to dig around for a bit to see if there are other more
general improvements like this to be had..

Thanks!

/Claes

[1]
Baseline:
Benchmark    Mode 
Cnt Score   Error  Units
UUIDBenchmark.benchmarkUUIDFromString    avgt 
4 0.505 ± 0.066  us/op

UUIDBenchmark.benchmarkUUIDFromString:CPI avgt 1.024   #/op
UUIDBenchmark.benchmarkUUIDFromString:L1-dcache-load-misses 
avgt  3.000   #/op
UUIDBenchmark.benchmarkUUIDFromString:L1-dcache-loads avgt 
211.128   #/op
UUIDBenchmark.benchmarkUUIDFromString:L1-dcache-stores avgt 
34.894   #/op
UUIDBenchmark.benchmarkUUIDFromString:L1-icache-load-misses 
avgt  0.067   #/op
UUIDBenchmark.benchmarkUUIDFromString:LLC-load-misses avgt  
0.103   #/op

UUIDBenchmark.benchmarkUUIDFromString:LLC-loads avgt 0.812   #/op
UUIDBenchmark.benchmarkUUIDFromString:LLC-store-misses avgt  
0.004   #/op
UUIDBenchmark.benchmarkUUIDFromString:LLC-stores avgt 0.054   
#/op
UUIDBenchmark.benchmarkUUIDFromString:branch-misses avgt 
13.269   #/op
UUIDBenchmark.benchmarkUUIDFromString:branches avgt 301.931   
#/op

UUIDBenchmark.benchmarkUUIDFromString:cycles avgt 1585.057   #/op
UUIDBenchmark.benchmarkUUIDFromString:dTLB-load-misses avgt  
0.005   #/op
UUIDBenchmark.benchmarkUUIDFromString:dTLB-loads avgt 
208.788   #/op
UUIDBenchmark.benchmarkUUIDFromString:dTLB-store-misses avgt ≈ 
10⁻⁴   #/op
UUIDBenchmark.benchmarkUUIDFromString:dTLB-stores avgt 
30.540   #/op
UUIDBenchmark.benchmarkUUIDFromString:iTLB-load-misses avgt  
0.002   #/op
UUIDBenchmark.benchmarkUUIDFromString:iTLB-loads avgt 0.006   
#/op
UUIDBenchmark.benchmarkUUIDFromString:instructions avgt 
1547.568   #/op


Results with 
http://cr.openjdk.java.net/~redestad/scratch/char.digit.00/ -- :
Benchmark    Mode 
Cnt Score   Error  Units
UUIDBenchmark.benchmarkUUIDFromString    avgt 
4 0.310 ± 0.022  us/op

UUIDBenchmark.benchmarkUUIDFromString:CPI avgt 0.832   #/op
UUIDBenchmark.benchmarkUUIDFromString:L1-dcache-load-misses 
avgt  2.026   #/op
UUIDBenchmark.benchmarkUUIDFromString:L1-dcache-loads avgt 
208.227   #/op
UUIDBenchmark.benchmarkUUIDFromString:L1-dcache-stores avgt 
30.673   #/op
UUIDBenchmark.benchmarkUUIDFromString:L1-icache-load-misses 
avgt  0.052   #/op
UUIDBenchmark.benchmarkUUIDFromString:LLC-load-misses avgt  
0.042   #/op

UUIDBenchmark.benchmarkUUIDFromString:LLC-loads avgt 1.027   #/op
UUIDBenchmark.benchmarkUUIDFromString:LLC-store-misses avgt  
0.007   

Re: [JDK 11] RFR 8196211: Move two sun/nio/cs tests into OpenJDK

[Amy Lu]

Updated on the inputFileName.

Please review: http://cr.openjdk.java.net/~amlu/8196211/webrev.01/

Thanks,
Amy

On 27/01/2018 2:32 AM, Paul Sandoz wrote:

Hi,

Quick observation.

EUCTWBufferBoundaryDecodeTest uses a different data file on windows and it uses 
the line separator as a trigger. Is it possible to better formalize this by 
passing in the argument for the file via jtreg?

Paul.



On Jan 25, 2018, at 11:45 PM, Amy Lu  wrote:

Please review the patch to move two sun/nio/cs tests into OpenJDK.

bug: https://bugs.openjdk.java.net/browse/JDK-8196211
webrev: http://cr.openjdk.java.net/~amlu/8196211/webrev.00/

Thanks,
Amy

Re: Clarification for adding classes shipped with JDK to sun.rmi.registry.registryFilter property in java.security file

[Nasser Ebrahim]

Hi Roger,
 
Thank you for your inputs. I agree with you that logging the filtering 
action can help developers to easily identify the classes which are failed 
due to the new filtering mechanism.
 
However, it may not be fair to ask the application developers to add the 
basic JDK classes when Java introduced the filtering mechanism. For 
application classes, it makes sense for developer to use the logging 
mechanism to identify the classes to be added to the white list.
 
I think JDK should add all its standard classes (at least the base 
packages) to white list by default rather than asking each end user to add 
to the white list.
 
Please clarify whether there is any reason for not adding the standard JDK 
classes by default to the white list as they are trusted code.
 
Thank you,
Nasser Ebrahim



From:   Roger Riggs 
To: core-libs-dev@openjdk.java.net
Date:   01/19/2018 08:50 PM
Subject:Re: Clarification for adding classes shipped with JDK to 
sun.rmi.registry.registryFilter property in java.security file
Sent by:"core-libs-dev" 



Hi Vipin,

A couple of suggestions to make working with serialization filters easier.

JEP 290 enabled logging of filtering actions by setting 
java.io.serialization.level =  FINEST.
Add that to the logging.properties and supply it on the command line with
"-Djava.util.logging.config.file=logging.properties".
If set to FINEST, it will log every call to the filter with the 
arguments of class, array size, etc.
If set to FINE, it will log only the cases where the filter rejects 
based on its arguments.

The pattern based filters can allow or disallow entire packages.
For example, "java.net.**" will allow any class in any package with a 
"java.net." prefix.

For development purposes, serialized classes for the registry could be 
identified using a filter of "*"
with logging enabled as FINEST and examining the log.

Expanding the white list for the registry is a possibility with a very 
conservative
evaluation of specific cases.

Regards, Roger

[1] 
https://urldefense.proofpoint.com/v2/url?u=http-3A__openjdk.java.net_jeps_290&d=DwIFaQ&c=jf_iaSHvJObTbx-siA1ZOg&r=yiMdG4h7wT1IHGC_YAWYv5USAuyCyINm1bo82D5y5bY&m=12RWeCnDad-pQO_PzN0K_mAliSGWmNyH_hOob40MLlA&s=HSk5hl04g1T39ZWtVoEpYFo0CYAkuLPyfQlKA2eh5Us&e=



On 1/16/2018 7:21 AM, Vipin Mv1 wrote:
>
> Hi,
>
> After upgrading to oracle 8u121-b12, I get following Exception when 
running
> the following testcase.
>
> import java.io.*;
> import java.net.InetAddress;
> import java.rmi.*;
> import java.rmi.registry.*;
> import java.security.Security;
>
> /* @test
>   * @run main/othervm RegistryFilterTest
>   */
>
> public class RegistryFilterTest {
>
>  private static int port = 12345;
>  private static Registry registry;
>
>  static class RemoteObj implements Serializable, Remote {
>  private static final long serialVersionUID = 01L;
>
>  final Object obj;
>
>  RemoteObj(Object obj) {
>  this.obj = obj;
>  }
>
>
>  }
>
>
>  public static void main(String[] args) throws Exception{
>
>  InetAddress in = InetAddress.getLocalHost();
>  LocateRegistry.createRegistry(port);
>  Registry registry = LocateRegistry.getRegistry("localhost",
> port);
>  registry.bind("InetAddress", new RemoteObj(in));
>  registry.unbind("InetAddress");
>  System.out.println("RMI Registry Test Passed");
>
>  }
>
> }
>
> TestResults
> ---
> Jan 12, 2018 5:32:18 PM java.io.ObjectInputStream filterCheck
> INFO: ObjectInputFilter REJECTED: class java.net.InetAddress, array 
length:
> -1, nRefs: 5, depth: 2, bytes: 216, ex: n/a
> Caused by: java.io.InvalidClassException: filter status: REJECTED
>  at 
java.io.ObjectInputStream.filterCheck(ObjectInputStream.java:1406)
>  at java.io.ObjectInputStream.readNonProxyDesc
> (ObjectInputStream.java:1997)
>  at 
java.io.ObjectInputStream.readClassDesc(ObjectInputStream.java:1877)
>  at java.io.ObjectInputStream.readOrdinaryObject
> (ObjectInputStream.java:2170)
>  at 
java.io.ObjectInputStream.readObject0(ObjectInputStream.java:1698)
>  at java.io.ObjectInputStream.defaultReadFields
> (ObjectInputStream.java:2415)
>  at java.io.ObjectInputStream.readSerialData
> (ObjectInputStream.java:2339)
>  at java.io.ObjectInputStream.readOrdinaryObject
> (ObjectInputStream.java:2197)
>  at 
java.io.ObjectInputStream.readObject0(ObjectInputStream.java:1698)
>  at 
java.io.ObjectInputStream.readObjectImpl(ObjectInputStream.java:540)
>  at java.io.ObjectInputStream.readObject(ObjectInputStream.java:475)
>  ... 16 more
>
> I understand it is due to the new serialization Filtering mechanism
> introduced to RMI Registry and Distributed Garbage Collection as a part 
of
> the following fixes.
>
> JDK-8160108 Implement Serialization Filtering
> JDK-8156804: Bet

Re: Optimizing UUID#fromString(String)

[Jon Chambers]

Awesome. Excited to see the Character.digit improvements!

To set expectations from here, I'll need to do two things to move forward:

1. I need to sign the OCA, as you've pointed out. As a courtesy, I'd like
to alert my employer before doing so. This has been a non-issue for similar
agreements for other open-source projects in the past.
2. I'll need to work through the OpenJDK build/test process and (I
suspect?) do some very preliminary reading on Mercurial. Because this is a
personal project, I may not have an opportunity to make much headway there
until this weekend.

That said, I'm grateful for the sponsorship and look forward to working
with you. I'll follow up directly to discuss the details.

Cheers!

-Jon

On Mon, Jan 29, 2018 at 6:30 AM, Claes Redestad 
wrote:

> Hi,
>
> I've filed two RFEs:
>
> - https://bugs.openjdk.java.net/browse/JDK-8196331 for providing a
>   fast-path for ASCII/Latin1 input to Character.digit. Patch out
>   for review
> - https://bugs.openjdk.java.net/browse/JDK-8196334 for tracking
>   the optimizations specific to UUID proposed here
>
> Jon, if you want to provide a patch I'd be happy to sponsor. I have
> a few ideas on how to retrofit existing behavior that we can discuss
> offline. Also, if you haven't filed an OCA then this might be a good
> time to do so[1]
>
> Thanks!
>
> /Claes
>
> [1] http://openjdk.java.net/contribute/
>
>
> On 2018-01-29 00:19, Claes Redestad wrote:
>
>> Hi Jon,
>>
>> On 2018-01-27 17:05, Jon Chambers wrote:
>>
>>> Regardless, I wanted to call this optimization opportunity to your
>>> attention, and would be happy to offer a proper patch if this seems like
>>> a
>>> worthwhile change.
>>>
>>
>> this does looks promising - and at least points out there might be
>> some performance issues lurking here!
>>
>> Found another compatibility issue, however: The current
>> UUID.fromString allow use of any unicode digits parseable into
>> digits. Say, Arabic digits (U+0660 - U+0669):
>>
>> jshell> UUID.fromString("\u0669-\u0669-\u0669-\u0669-\u0669");
>> $2 ==> 0009-0009-0009-0009-0009
>>
>> That you're getting some improvement from avoiding
>> Character.digit(..) stands out in a way, so I wondered if there's
>> something going on here... and it does seem that this method could
>> be optimized by providing a fast-path in the CharacterDataLatin1
>> plane...
>>
>> I experimented with a few solutions, and the best I could find was
>> a 256 element byte[] lookup table for digits in CharacterDataLatin1
>> (spanning the entire 8-bit space of CharacterDataLatin1 allows us
>> to avoid a branch):
>>
>> http://cr.openjdk.java.net/~redestad/scratch/char.digit.00/
>>
>> This speeds up your micro testing java.util.UUID
>> (UUIDBenchmark.benchmarkUUIDFromString) by ~1.6x. The
>> improvement is there on all counters when looking at -prof perfnorm
>> (less loads, stores, branches, instructions, better CPI etc)[1], so seems
>> like a pretty straightforward thing to consider independently.
>>
>> Your FastUUID implementation is still quite a bit faster, though, but
>> it'd be good to dig around for a bit to see if there are other more
>> general improvements like this to be had..
>>
>> Thanks!
>>
>> /Claes
>>
>> [1]
>> Baseline:
>> BenchmarkMode
>> Cnt Score   Error  Units
>> UUIDBenchmark.benchmarkUUIDFromStringavgt 4
>> 0.505 ± 0.066  us/op
>> UUIDBenchmark.benchmarkUUIDFromString:CPI avgt 1.024   #/op
>> UUIDBenchmark.benchmarkUUIDFromString:L1-dcache-load-misses
>> avgt  3.000   #/op
>> UUIDBenchmark.benchmarkUUIDFromString:L1-dcache-loads avgt
>> 211.128   #/op
>> UUIDBenchmark.benchmarkUUIDFromString:L1-dcache-stores avgt
>> 34.894   #/op
>> UUIDBenchmark.benchmarkUUIDFromString:L1-icache-load-misses
>> avgt  0.067   #/op
>> UUIDBenchmark.benchmarkUUIDFromString:LLC-load-misses avgt
>> 0.103   #/op
>> UUIDBenchmark.benchmarkUUIDFromString:LLC-loads avgt 0.812   #/op
>> UUIDBenchmark.benchmarkUUIDFromString:LLC-store-misses avgt
>> 0.004   #/op
>> UUIDBenchmark.benchmarkUUIDFromString:LLC-stores avgt 0.054
>> #/op
>> UUIDBenchmark.benchmarkUUIDFromString:branch-misses avgt
>> 13.269   #/op
>> UUIDBenchmark.benchmarkUUIDFromString:branches avgt 301.931
>> #/op
>> UUIDBenchmark.benchmarkUUIDFromString:cycles avgt 1585.057   #/op
>> UUIDBenchmark.benchmarkUUIDFromString:dTLB-load-misses avgt
>> 0.005   #/op
>> UUIDBenchmark.benchmarkUUIDFromString:dTLB-loads avgt 208.788
>> #/op
>> UUIDBenchmark.benchmarkUUIDFromString:dTLB-store-misses avgt ≈
>> 10⁻⁴   #/op
>> UUIDBenchmark.benchmarkUUIDFromString:dTLB-stores avgt 30.540
>> #/op
>> UUIDBenchmark.benchmarkUUIDFromString:iTLB-load-misses avgt
>> 0.002   #/op
>> UUIDBenchmark.benchmarkUUIDFromString:iTLB-loads avgt 0.006
>> #/op
>> UUIDBenchmark.benchmarkUUIDFromString:instructions avgt
>> 1547.568   

Re: RFR JDK-8194412 : Adding 256 units of IsoFields.QUARTER_YEARS broken

[Roger Riggs]

Hi Ivan,

Looks good!

Thanks, Roger


On 1/28/2018 6:09 PM, Stephen Colebourne wrote:

Spelling mistake in tests "qarterYears"
Otherwise, looks good.
thanks
Stephen



On 27 January 2018 at 19:37, Ivan Gerasimov  wrote:

Hello!

It was spotted that adding more then 255 quarter-years to a Temporal leads
to a wrong result.

BUGURL: https://bugs.openjdk.java.net/browse/JDK-8194412
WEBREV: http://cr.openjdk.java.net/~igerasim/8194412/00/webrev/

Would you please take a look at the fix and see that I'm not missing
something?

Thanks in advance!

--
With kind regards,
Ivan Gerasimov

Re: RFR[JDK10]: 8195837: (tz) Support tzdata2018c

[Seán Coffey]

The changes look fine to me Ramanand. The europe file contains some 
interesting comments about the rolled back changes that have been made 
for 2018c. A plan on how to resolve these pending changes can be 
followed up via JDK-8195595


Regards,
Sean.

On 29/01/18 08:49, Ramanand Patil wrote:

Hi all,
Please review the latest TZDATA integration (tzdata2018c) into JDK10.
Bug: https://bugs.openjdk.java.net/browse/JDK-8195837
Webrev: http://cr.openjdk.java.net/~rpatil/8195837/webrev.00/

All the TimeZone related tests are passed after integration.

Regards,
Ramanand.

Re: JDK-8196298: Add null Reader and Writer

[Brian Burkhalter]

Hi Patrick,

Conceptually this looks good. There might be some javadoc tweaks needed but 
those can be worked out while this is a draft CSR (to be filed).

Thanks,

Brian

On Jan 27, 2018, at 12:45 AM, Patrick Reinhart  wrote:

> I saw that great extension on InputStream/OutputStream adding a Null
> implementation there. Based on those I would propose a similar addition
> to the Reader/Writer.

Re: RFR[JDK10]: 8195837: (tz) Support tzdata2018c

[Andrew Hughes]

On 29 January 2018 at 14:42, Seán Coffey  wrote:
> The changes look fine to me Ramanand. The europe file contains some
> interesting comments about the rolled back changes that have been made for
> 2018c. A plan on how to resolve these pending changes can be followed up via
> JDK-8195595
>

Yes... I've been following the discussion on the tzdb list about these
changes to the
Irish timezone and the breakage they cause. It's good to see that, at
least for now,
these changes will be reverted locally for OpenJDK.

My thanks to those who've put forward the case on that list for the
compatibility issues
these changes cause, particularly Stephen Colebourne.

> Regards,
> Sean.
>
>

-- 
Andrew :)

Senior Free Java Software Engineer
Red Hat, Inc. (http://www.redhat.com)

Web Site: http://fuseyism.com
Twitter: https://twitter.com/gnu_andrew_java
PGP Key: ed25519/0xCFDA0F9B35964222 (hkp://keys.gnupg.net)
Fingerprint = 5132 579D D154 0ED2 3E04  C5A0 CFDA 0F9B 3596 4222

Re: [JDK 11] RFR 8196211: Move two sun/nio/cs tests into OpenJDK

[Paul Sandoz]

That’s better :-)

If you wanna make it static i suggest:

  s/inputFileName/INPUT_FILE_NAME

although there is little benefit here since all access is local to main. Up to 
you to make local or keep static. No need for another review.

Paul.

> On Jan 29, 2018, at 4:17 AM, Amy Lu  wrote:
> 
> Updated on the inputFileName.
>  Please review: http://cr.openjdk.java.net/~amlu/8196211/webrev.01/ 
> 
> 
> Thanks,
> Amy
> On 27/01/2018 2:32 AM, Paul Sandoz wrote:
>> Hi,
>> 
>> Quick observation.
>> 
>> EUCTWBufferBoundaryDecodeTest uses a different data file on windows and it 
>> uses the line separator as a trigger. Is it possible to better formalize 
>> this by passing in the argument for the file via jtreg?
>> 
>> Paul.
>> 
>> 
>>> On Jan 25, 2018, at 11:45 PM, Amy Lu  
>>>  wrote:
>>> 
>>> Please review the patch to move two sun/nio/cs tests into OpenJDK.
>>> 
>>> bug: https://bugs.openjdk.java.net/browse/JDK-8196211 
>>> 
>>> webrev: http://cr.openjdk.java.net/~amlu/8196211/webrev.00/ 
>>> 
>>> 
>>> Thanks,
>>> Amy
> 

Re: RFR[JDK10]: 8195837: (tz) Support tzdata2018c

[Martin Buchholz]

Google has been successfully running jdk8 and jdk9 with 2018c.

On Mon, Jan 29, 2018 at 12:49 AM, Ramanand Patil 
wrote:

> Hi all,
> Please review the latest TZDATA integration (tzdata2018c) into JDK10.
> Bug: https://bugs.openjdk.java.net/browse/JDK-8195837
> Webrev: http://cr.openjdk.java.net/~rpatil/8195837/webrev.00/
>
> All the TimeZone related tests are passed after integration.
>
> Regards,
> Ramanand.
>

Re: RFR: 8196331: Optimize Character.digit for latin1 input

[Martin Buchholz]

Thanks.  I might try to shrink the size of the static array, by only
storing values between '0' and 'z', at the cost of slightly greater lookup
costs for each char.

On Mon, Jan 29, 2018 at 3:15 AM, Claes Redestad 
wrote:

> Hi,
>
> for the latin1 block of CharacterData we can improve the
> digit(int, int) implementation by providing an optimized lookup
> table. This improves microbenchmarks exercising Integer.parseInt,
> Long.parseLong and UUID.fromString etc by around 50%for typical
> inputs.
>
> Webrev: http://cr.openjdk.java.net/~redestad/8196331/open.00/
> Bug: https://bugs.openjdk.java.net/browse/JDK-8196331
>
> The lookup array is pre-calculated to minimize startup impact
> (adds 1,027 bytecodes executed during initialization)
>
> /Claes
>

Re: RFR JDK-8176379: java.util.Base64 mime encoder behaves incorrectly if initialized with a line length of size 1-3

[Xueming Shen]

On 01/26/2018 03:53 PM, Paul Sandoz wrote:

Base64
—

138 lineLength = lineLength>>  2<<  2;

I know the code was shuffled up but i find the following clearer in terms of 
bit manipulation:

   // Round down to nearest mulitple of 4
   lineLength&= ~0b11



sure, we can go the straightforward bit manipulation.


TestBase64
—

Is there a test passing in a negative value for len?



Actually I have a dedicated test case for "mime encoder maxlen".
*test/jdk/java/util/Base64/Base64GetEncoderTest.java*
I have updated it to test the negative and the < 4 maxlen.

http://cr.openjdk.java.net/~sherman/8176379/webrev/

webrev has been updated according.

Thanks!
Sherman

Re: RFR: 8196331: Optimize Character.digit for latin1 input

[Paul Sandoz]

> On Jan 29, 2018, at 9:44 AM, Martin Buchholz  wrote:
> 
> Thanks.  I might try to shrink the size of the static array, by only
> storing values between '0' and 'z', at the cost of slightly greater lookup
> costs for each char.
> 

I was wondering the same, or just clip the end of the array to’z'

if (ch <= ‘z’ && radix …) { // Might even fold the upper bounds check for DIGITS
  value = DIGITS[ch];
  ...
}

Paul.

> On Mon, Jan 29, 2018 at 3:15 AM, Claes Redestad 
> wrote:
> 
>> Hi,
>> 
>> for the latin1 block of CharacterData we can improve the
>> digit(int, int) implementation by providing an optimized lookup
>> table. This improves microbenchmarks exercising Integer.parseInt,
>> Long.parseLong and UUID.fromString etc by around 50%for typical
>> inputs.
>> 
>> Webrev: http://cr.openjdk.java.net/~redestad/8196331/open.00/
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8196331
>> 
>> The lookup array is pre-calculated to minimize startup impact
>> (adds 1,027 bytecodes executed during initialization)
>> 
>> /Claes
>> 

Re: RFR JDK-8176379: java.util.Base64 mime encoder behaves incorrectly if initialized with a line length of size 1-3

[Paul Sandoz]

+1

We will require a CSR for the minor adjustment to the specification.

Paul.

> On Jan 29, 2018, at 9:52 AM, Xueming Shen  wrote:
> 
> On 01/26/2018 03:53 PM, Paul Sandoz wrote:
>> 
>> Base64
>> —
>> 
>> 138 lineLength = lineLength >> 2 << 2;
>> 
>> I know the code was shuffled up but i find the following clearer in terms of 
>> bit manipulation:
>> 
>>   // Round down to nearest mulitple of 4
>>   lineLength &= ~0b11
>> 
> 
> sure, we can go the straightforward bit manipulation.
> 
>> TestBase64
>> —
>> 
>> Is there a test passing in a negative value for len?
>> 
> 
> Actually I have a dedicated test case for "mime encoder maxlen".
> test/jdk/java/util/Base64/Base64GetEncoderTest.java 
> I have updated it to test the negative and the < 4 maxlen.
> 
> http://cr.openjdk.java.net/~sherman/8176379/webrev/ 
> 
> 
> webrev has been updated according.
> 
> Thanks!
> Sherman

Re: RFR JDK-8176379: java.util.Base64 mime encoder behaves incorrectly if initialized with a line length of size 1-3

[Roger Riggs]

Hi,

Looks good.

Base64.java: line 120  typo:  "outputut" -> "output"

Roger


On 1/29/2018 1:20 PM, Paul Sandoz wrote:

+1

We will require a CSR for the minor adjustment to the specification.

Paul.


On Jan 29, 2018, at 9:52 AM, Xueming Shen  wrote:

On 01/26/2018 03:53 PM, Paul Sandoz wrote:

Base64
—

138 lineLength = lineLength >> 2 << 2;

I know the code was shuffled up but i find the following clearer in terms of 
bit manipulation:

   // Round down to nearest mulitple of 4
   lineLength &= ~0b11


sure, we can go the straightforward bit manipulation.


TestBase64
—

Is there a test passing in a negative value for len?


Actually I have a dedicated test case for "mime encoder maxlen".
test/jdk/java/util/Base64/Base64GetEncoderTest.java
I have updated it to test the negative and the < 4 maxlen.

http://cr.openjdk.java.net/~sherman/8176379/webrev/ 


webrev has been updated according.

Thanks!
Sherman

JDK-8196298: Add null Reader and Writer

[Claudia Reinhart]

Hi there,

I saw that great extension on InputStream/OutputStream adding a Null
implementation there. Based on those I would propose a similar addition
to the Reader/Writer.

-Patrick


Here is what the API would look like based on the existing from
InputStream/OutputStream:


    /**
 * Returns a new {@code Reader} that contains no characters. The
returned
 * stream is initially open.  The stream is closed by calling the
 * {@code close()} method.  Subsequent calls to {@code close()} have no
 * effect.
 *
 *  While the stream is open, the {@code ready​()}, {@code read()},
 * {@code read(char[])}, {@code read(char[], int, int)}, {@code
skip()}, and
 * {@code transferTo()} methods all behave as if end of stream has been
 * reached.  After the stream has been closed, these methods all throw
 * {@code IOException}.
 *
 *  The {@code markSupported()} method returns {@code false}.  The
 * {@code mark()} method does nothing, and the {@code reset()} method
 * throws {@code IOException}.
 *
 * @return an {@code Reader} which contains no characters
 *
 * @since 11
 */
    public static Reader nullReader() {}


    /**
 * Returns a new {@code Writer} which discards all characters.  The
 * returned stream is initially open.  The stream is closed by calling
 * the {@code close()} method.  Subsequent calls to {@code close()} have
 * no effect.
 *
 *  While the stream is open, the {@code write(int)}, {@code
 * write(char[])}, and {@code write(char[], int, int)} methods do
nothing.
 * After the stream has been closed, these methods all throw {@code
 * IOException}.
 *
 *  The {@code flush()} method does nothing.
 *
 * @return an {@code Writer} which discards all characters
 *
 * @since 11
 */
    public static Writer nullWriter() {}

Re: RFR: 8196331: Optimize Character.digit for latin1 input

[Claes Redestad]

I ran with a smaller byte[] initially and saw about a 10% improvement from 
removing the branch, which is why I felt the superfluous bytes were motivated.

/Claes

Paul Sandoz  skrev: (29 januari 2018 19:14:44 CET)
>
>
>> On Jan 29, 2018, at 9:44 AM, Martin Buchholz 
>wrote:
>> 
>> Thanks.  I might try to shrink the size of the static array, by only
>> storing values between '0' and 'z', at the cost of slightly greater
>lookup
>> costs for each char.
>> 
>
>I was wondering the same, or just clip the end of the array to’z'
>
>if (ch <= ‘z’ && radix …) { // Might even fold the upper bounds check
>for DIGITS
>  value = DIGITS[ch];
>  ...
>}
>
>Paul.
>
>> On Mon, Jan 29, 2018 at 3:15 AM, Claes Redestad
>
>> wrote:
>> 
>>> Hi,
>>> 
>>> for the latin1 block of CharacterData we can improve the
>>> digit(int, int) implementation by providing an optimized lookup
>>> table. This improves microbenchmarks exercising Integer.parseInt,
>>> Long.parseLong and UUID.fromString etc by around 50%for typical
>>> inputs.
>>> 
>>> Webrev: http://cr.openjdk.java.net/~redestad/8196331/open.00/
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8196331
>>> 
>>> The lookup array is pre-calculated to minimize startup impact
>>> (adds 1,027 bytecodes executed during initialization)
>>> 
>>> /Claes
>>> 

-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

Re: RFR: 8196331: Optimize Character.digit for latin1 input

[Paul Sandoz]

Smaller in only the upper bound? I would an explicit upper bounds check would 
dominate that of the bounds check for the array itself.

Paul.

> On Jan 29, 2018, at 11:37 AM, Claes Redestad  
> wrote:
> 
> I ran with a smaller byte[] initially and saw about a 10% improvement from 
> removing the branch, which is why I felt the superfluous bytes were motivated.
> 
> /Claes
> 
> Paul Sandoz  skrev: (29 januari 2018 19:14:44 CET)
> 
> 
>  On Jan 29, 2018, at 9:44 AM, Martin Buchholz  wrote:
>  
>  Thanks.  I might try to shrink the size of the static array, by only
>  storing values between '0' and 'z', at the cost of slightly greater lookup
>  costs for each char.
>  
> 
> I was wondering the same, or just clip the end of the array to’z'
> 
> if (ch <= ‘z’ && radix …) { // Might even fold the upper bounds check for 
> DIGITS
>   value = DIGITS[ch];
>   ...
> }
> 
> Paul.
> 
>  On Mon, Jan 29, 2018 at 3:15 AM, Claes Redestad 
>  wrote:
>  
>  Hi,
>  
>  for the latin1 block of CharacterData we can improve the
>  digit(int, int) implementation by providing an optimized lookup
>  table. This improves microbenchmarks exercising Integer.parseInt,
>  Long.parseLong and UUID.fromString etc by around 50%for typical
>  inputs.
>  
>  Webrev: http://cr.openjdk.java.net/~redestad/8196331/open.00/ 
> 
>  Bug: https://bugs.openjdk.java.net/browse/JDK-8196331 
> 
>  
>  The lookup array is pre-calculated to minimize startup impact
>  (adds 1,027 bytecodes executed during initialization)
>  
>  /Claes
>  
> 
> 
> -- 
> Sent from my Android device with K-9 Mail. Please excuse my brevity.

Re: JDK-8196298: Add null Reader and Writer

[Patrick Reinhart]

Just added a new CSR for it based on the one for InputStream/OutputStream...

-Patrick

Am 29.01.2018 um 17:14 schrieb Brian Burkhalter:
> Hi Patrick,
>
> Conceptually this looks good. There might be some javadoc tweaks
> needed but those can be worked out while this is a draft CSR (to be
> filed).
>
> Thanks,
>
> Brian
>
> On Jan 27, 2018, at 12:45 AM, Patrick Reinhart  > wrote:
>
>> I saw that great extension on InputStream/OutputStream adding a Null
>> implementation there. Based on those I would propose a similar addition
>> to the Reader/Writer.
>

Re: JDK-8196298: Add null Reader and Writer

[Brian Burkhalter]

Great - thanks - will take a look.

Brian

On Jan 29, 2018, at 11:43 AM, Patrick Reinhart  wrote:

> Just added a new CSR for it based on the one for InputStream/OutputStream...

Re: RFR[JDK10]: 8195837: (tz) Support tzdata2018c

[Naoto Sato]

Looks good to me.

Naoto

On 1/29/18 12:49 AM, Ramanand Patil wrote:

Hi all,
Please review the latest TZDATA integration (tzdata2018c) into JDK10.
Bug: https://bugs.openjdk.java.net/browse/JDK-8195837
Webrev: http://cr.openjdk.java.net/~rpatil/8195837/webrev.00/

All the TimeZone related tests are passed after integration.

Regards,
Ramanand.

Re: [PATCH] Inefficient ArrayList.subList().toArray()

[Martin Buchholz]

I'm going to expedite this a little since we are building further changes
on top.

RFR: jsr166 jdk integration 2018-02
http://cr.openjdk.java.net/~martin/webrevs/jdk/jsr166-integration/overview.html

8196207: Inefficient ArrayList.subList().toArray()
http://cr.openjdk.java.net/~martin/webrevs/jdk/jsr166-integration/ArrayList-subList-toArray/index.html
https://bugs.openjdk.java.net/browse/JDK-8196207


On Fri, Jan 26, 2018 at 9:12 AM, Martin Buchholz 
wrote:

> It may be that array-backed lists inheriting from AbstractList is not
> useful except when creating a prototype (or as a way of testing
> AbstractList!) because a high-quality implementation should always override.
>
> On Fri, Jan 26, 2018 at 5:31 AM, Claes Redestad  > wrote:
>
>> On 2018-01-26 07:20, Martin Buchholz wrote:
>>
>>> It's still possible to find simple performance improvements in classes
>>> like
>>> ArrayList.
>>>
>>
>> Indeed!
>>
>> It seems ArrayList.SubList delegates quite a few of its implementations
>> to AbstractList and AbstractCollection, which uses iterators for things
>> that could be done more efficiently with conventional for-loops.
>>
>> This includes contains, equals, hashCode, indexOf, lastIndexOf... I saw
>> up to 7x improvements on some of these over the inherited
>> implementations when providing specialized ones for ImmutableList.SubList
>> as part of JDK-8193128
>>
>> I'll file a bug and maybe even tinker with it when I find some time...
>>
>> /Claes
>>
>
>

Re: RFR: 8196331: Optimize Character.digit for latin1 input

[Claes Redestad]

Right, I can't really explain why, but the effect is very visible and
reproducible in microbenchmarks. Differences in generated ASM might
be indicating that the inlining behavior changes enough to shift the
result around; maybe a job for @ForceInline?

I'll experiment and analyze a bit more tomorrow to see if I can find a
good explanation for the observed difference and/or a solution that
allows us to slim down the lookup array.

/Claes

On 2018-01-29 20:38, Paul Sandoz wrote:
Smaller in only the upper bound? I would an explicit upper bounds 
check would dominate that of the bounds check for the array itself.


Paul.

On Jan 29, 2018, at 11:37 AM, Claes Redestad 
mailto:claes.redes...@oracle.com>> wrote:


I ran with a smaller byte[] initially and saw about a 10% improvement 
from removing the branch, which is why I felt the superfluous bytes 
were motivated.


/Claes

Paul Sandoz mailto:paul.san...@oracle.com>> 
skrev: (29 januari 2018 19:14:44 CET)



On Jan 29, 2018, at 9:44 AM, Martin Buchholz
mailto:marti...@google.com>> wrote:
Thanks. I might try to shrink the size of the static array,
by only storing values between '0' and 'z', at the cost of
slightly greater lookup costs for each char. 



I was wondering the same, or just clip the end of the array to’z'

if (ch <= ‘z’ && radix …) { // Might even fold the upper bounds check for 
DIGITS
   value = DIGITS[ch];
   ...
}

Paul.

On Mon, Jan 29, 2018 at 3:15 AM, Claes Redestad
mailto:claes.redes...@oracle.com>> wrote:

Hi, for the latin1 block of CharacterData we can improve
the digit(int, int) implementation by providing an
optimized lookup table. This improves microbenchmarks
exercising Integer.parseInt, Long.parseLong and
UUID.fromString etc by around 50%for typical inputs.
Webrev:
http://cr.openjdk.java.net/~redestad/8196331/open.00/

Bug: https://bugs.openjdk.java.net/browse/JDK-8196331 The
lookup array is pre-calculated to minimize startup impact
(adds 1,027 bytecodes executed during initialization) /Claes 




--
Sent from my Android device with K-9 Mail. Please excuse my brevity.

Re: JDK-8196298: Add null Reader and Writer

[Brian Burkhalter]

I made a few minor changes to the CSR [1] verbiage. Probably the nullWriter() 
documentation should also be updated to explicitly mention all variants of both 
append() and write().

Thanks,

Brian

[1] https://bugs.openjdk.java.net/browse/JDK-8196350

On Jan 29, 2018, at 12:58 PM, Brian Burkhalter  
wrote:

> Great - thanks - will take a look.
> 
> Brian
> 
> On Jan 29, 2018, at 11:43 AM, Patrick Reinhart  wrote:
> 
>> Just added a new CSR for it based on the one for InputStream/OutputStream...

Re: [PATCH] Inefficient ArrayList.subList().toArray()

[Paul Sandoz]

> On Jan 29, 2018, at 1:02 PM, Martin Buchholz  wrote:
> 
> I'm going to expedite this a little since we are building further changes
> on top.
> 
> RFR: jsr166 jdk integration 2018-02
> http://cr.openjdk.java.net/~martin/webrevs/jdk/jsr166-integration/overview.html
> 

Looks ok, but i personally would not categorize the F/J changes as 
miscellaneous :-)


> 8196207: Inefficient ArrayList.subList().toArray()
> http://cr.openjdk.java.net/~martin/webrevs/jdk/jsr166-integration/ArrayList-subList-toArray/index.html
> https://bugs.openjdk.java.net/browse/JDK-8196207
> 

Looks ok too.

Paul.

Re: [PATCH] Inefficient ArrayList.subList().toArray()

[Martin Buchholz]

On Mon, Jan 29, 2018 at 3:10 PM, Paul Sandoz  wrote:

>
>
> > On Jan 29, 2018, at 1:02 PM, Martin Buchholz 
> wrote:
> >
> > I'm going to expedite this a little since we are building further changes
> > on top.
> >
> > RFR: jsr166 jdk integration 2018-02
> > http://cr.openjdk.java.net/~martin/webrevs/jdk/jsr166-
> integration/overview.html
> >
>
> Looks ok, but i personally would not categorize the F/J changes as
> miscellaneous :-)
>

Sorry, we're still working on forkjoin; only the ArrayList changes are
ready to go.

Re: [PATCH] Inefficient ArrayList.subList().toArray()

[Paul Sandoz]

> On Jan 29, 2018, at 3:30 PM, Martin Buchholz  wrote:
> 
> 
> 
> On Mon, Jan 29, 2018 at 3:10 PM, Paul Sandoz  > wrote:
> 
> 
> > On Jan 29, 2018, at 1:02 PM, Martin Buchholz  > > wrote:
> >
> > I'm going to expedite this a little since we are building further changes
> > on top.
> >
> > RFR: jsr166 jdk integration 2018-02
> > http://cr.openjdk.java.net/~martin/webrevs/jdk/jsr166-integration/overview.html
> >  
> > 
> >
> 
> Looks ok, but i personally would not categorize the F/J changes as 
> miscellaneous :-)
> 
> Sorry, we're still working on forkjoin;

Ok, split from the misc changes?


> only the ArrayList changes are ready to go.

+1

Paul.

Re: [PATCH] Inefficient ArrayList.subList().toArray()

[John Rose]

Reviewed (officially).  This is a good point-fix.

— John

P.S.  I still think we have some tech. debt to discharge
in finding a way to generically provide zero-copy access
to array data, across interoperating collections APIs.

If we got the deeper, more general answer right, we would
be able to reformulate the present point-fix in terms of a
single viewing operation, and then apply it in other places
(like Arrays.asList) without more code duplication.

I think (though am not sure yet) that the Collection::stream
API point is the right place to inject zero-copy array viewing
capabilities.  A specialized array-backed Stream (SIZED,
of course) would provide a general but efficient connection
point, that would allow private arrays to be securely read
but not written.  Then the various array-backed implementations
(and their sublists) would simply override their respective
stream views.  The copyOfRange step would appear,
not in a bunch of point fixes, but centrally in the array-backed
stream code.  A package-private class could mediate
secret access to the backing array, if necessary.

See also this discussion about generic array-producing terminal
methods, near this message:
  http://mail.openjdk.java.net/pipermail/core-libs-dev/2017-December/050560.html

On Jan 29, 2018, at 3:30 PM, Martin Buchholz  wrote:
> 
> 
> 
> On Mon, Jan 29, 2018 at 3:10 PM, Paul Sandoz  > wrote:
> 
> 
> > On Jan 29, 2018, at 1:02 PM, Martin Buchholz  > > wrote:
> >
> > I'm going to expedite this a little since we are building further changes
> > on top.
> >
> > RFR: jsr166 jdk integration 2018-02
> > http://cr.openjdk.java.net/~martin/webrevs/jdk/jsr166-integration/overview.html
> >  
> > 
> >
> 
> Looks ok, but i personally would not categorize the F/J changes as 
> miscellaneous :-)
> 
> Sorry, we're still working on forkjoin; only the ArrayList changes are ready 
> to go.

Re: [PATCH] Inefficient ArrayList.subList().toArray()

[John Rose]

On Jan 29, 2018, at 4:00 PM, John Rose  wrote:
> 
> Then the various array-backed implementations
> (and their sublists) would simply override their respective
> stream views.  

BTW, this notion is more general than you might think.
It applies to collections, like the internal spined buffer,
which are backed by sequences of arrays.  You use
the spliterator of the array-backed stream to find
the "cracks" between the backing arrays.  After
subdividing appropriately, you get Arrays.copyOf
and its friends at the leaves of the outer loop.

As I said, I *think* this can work, and if it does
it will address the issue of duplicate or slow copies
more systematically than slowly expanding but
incomplete set of point-fixes, like the one under
review.

Re: RFR: 8196331: Optimize Character.digit for latin1 input

[Ulf Zibis]

Hi,

you may can construct the lookup table as a static String constant to 
slim down the footprint and treat it as a byte[] as we do in the Charset 
coders.


-Ulf


Am 29.01.2018 um 22:04 schrieb Claes Redestad:

I'll experiment and analyze a bit more tomorrow to see if I can find a
good explanation for the observed difference and/or a solution that
allows us to slim down the lookup array.

Re: Cannot add attribute into main attributes of a jar if there is no version

[Weijun Wang]

Ping again.

> On Jan 22, 2018, at 1:12 PM, Weijun Wang  wrote:
> 
> src/java.base/share/classes/java/util/jar/Attributes.java:
> 
>   329 @SuppressWarnings("deprecation")
>   330 void writeMain(DataOutputStream out) throws IOException
>   331 {
>   332 // write out the *-Version header first, if it exists
>   333 String vername = Name.MANIFEST_VERSION.toString();
>   334 String version = getValue(vername);
>   335 if (version == null) {
>   336 vername = Name.SIGNATURE_VERSION.toString();
>   337 version = getValue(vername);
>   338 }
>   339 
>   340 if (version != null) {
>   341 out.writeBytes(vername+": "+version+"\r\n");
>   342 }
>   343 
>   344 // write out all attributes except for the version
>   345 // we wrote out earlier
>   346 for (Entry e : entrySet()) {
>   347 String name = ((Name) e.getKey()).toString();
>   348 if ((version != null) && !(name.equalsIgnoreCase(vername))) 
> {
> 
> So, if there is no existing MANIFEST_VERSION or SIGNATURE_VERSION, then 
> version is null and the check above will be false for ever and any other 
> attribute cannot be written out.
> 
> Is this intended? If so, we can exit with an else block after line 342.
> 
> Thanks
> Max
> 
> p.s. I am writing a test and notice this.
> 
>   349 
>   350 StringBuffer buffer = new StringBuffer(name);
>   351 buffer.append(": ");
>   352 
>   353 String value = (String) e.getValue();
>   354 if (value != null) {
>   355 byte[] vb = value.getBytes("UTF8");
>   356 value = new String(vb, 0, 0, vb.length);
>   357 }
>   358 buffer.append(value);
>   359 
>   360 buffer.append("\r\n");
>   361 Manifest.make72Safe(buffer);
>   362 out.writeBytes(buffer.toString());
>   363 }
>   364 }
>   365 out.writeBytes("\r\n");
>   366 }
> 

Re: Cannot add attribute into main attributes of a jar if there is no version

[mandy chung]

On 1/29/18 4:22 PM, Weijun Wang wrote:

Ping again.


On Jan 22, 2018, at 1:12 PM, Weijun Wang  wrote:

src/java.base/share/classes/java/util/jar/Attributes.java:

   329  @SuppressWarnings("deprecation")
   330  void writeMain(DataOutputStream out) throws IOException
   331  {
   332  // write out the *-Version header first, if it exists
   333  String vername = Name.MANIFEST_VERSION.toString();
   334  String version = getValue(vername);
   335  if (version == null) {
   336  vername = Name.SIGNATURE_VERSION.toString();
   337  version = getValue(vername);
   338  }
   339  
   340  if (version != null) {
   341  out.writeBytes(vername+": "+version+"\r\n");
   342  }
   343  
   344  // write out all attributes except for the version
   345  // we wrote out earlier
   346  for (Entry e : entrySet()) {
   347  String name = ((Name) e.getKey()).toString();
   348  if ((version != null) && !(name.equalsIgnoreCase(vername))) 
{

So, if there is no existing MANIFEST_VERSION or SIGNATURE_VERSION, then version 
is null and the check above will be false for ever and any other attribute 
cannot be written out.

Is this intended? If so, we can exit with an else block after line 342.


From code inspection, I agree that this method is a nop if there is no 
Manifest-Version attribute or Signature-Attribute.  This can return 
quickly without iterating the entry set.   I don't see any issue to make 
it an else block.


This method is only called from Manifest::write method which does not 
mention Signature-Version but I don't have the history to tell if this 
is intended or not.


Mandy

Re: Cannot add attribute into main attributes of a jar if there is no version

[Weijun Wang]

> On Jan 30, 2018, at 8:57 AM, mandy chung  wrote:
> 
> 
> 
> On 1/29/18 4:22 PM, Weijun Wang wrote:
>> Ping again.
>> 
>> 
>>> On Jan 22, 2018, at 1:12 PM, Weijun Wang 
>>>  wrote:
>>> 
>>> src/java.base/share/classes/java/util/jar/Attributes.java:
>>> 
>>>   329   @SuppressWarnings("deprecation")
>>>   330   void writeMain(DataOutputStream out) throws IOException
>>>   331   {
>>>   332   // write out the *-Version header first, if it exists
>>>   333   String vername = Name.MANIFEST_VERSION.toString();
>>>   334   String version = getValue(vername);
>>>   335   if (version == null) {
>>>   336   vername = Name.SIGNATURE_VERSION.toString();
>>>   337   version = getValue(vername);
>>>   338   }
>>>   339   
>>>   340   if (version != null) {
>>>   341   out.writeBytes(vername+": "+version+"\r\n");
>>>   342   }
>>>   343   
>>>   344   // write out all attributes except for the version
>>>   345   // we wrote out earlier
>>>   346   for (Entry e : entrySet()) {
>>>   347   String name = ((Name) e.getKey()).toString();
>>>   348   if ((version != null) && 
>>> !(name.equalsIgnoreCase(vername))) {
>>> 
>>> So, if there is no existing MANIFEST_VERSION or SIGNATURE_VERSION, then 
>>> version is null and the check above will be false for ever and any other 
>>> attribute cannot be written out.
>>> 
>>> Is this intended? If so, we can exit with an else block after line 342.
>>> 
> 
> From code inspection, I agree that this method is a nop if there is no 
> Manifest-Version attribute or Signature-Attribute.  This can return quickly 
> without iterating the entry set.   I don't see any issue to make it an else 
> block.

On the other hand, if this is not intended we should fix line 348 and remove 
the version != null check. I cannot find a place saying a MANIFEST_VERSION or 
SIGNATURE_VERSION must be provided. Even if so, this should be an error and 
it's not a good idea to silently drop all other attributes in the main section.

Anyway, I filed https://bugs.openjdk.java.net/browse/JDK-8196371.

--Max

> 
> This method is only called from Manifest::write method which does not mention 
> Signature-Version but I don't have the history to tell if this is intended or 
> not.
> 
> Mandy
> 
> 

Re: Cannot add attribute into main attributes of a jar if there is no version

[Xueming Shen]

On 1/29/18, 4:57 PM, mandy chung wrote:



On 1/29/18 4:22 PM, Weijun Wang wrote:

Ping again.

On Jan 22, 2018, at 1:12 PM, Weijun Wang  
wrote:


src/java.base/share/classes/java/util/jar/Attributes.java:

   329@SuppressWarnings("deprecation")
   330void writeMain(DataOutputStream out) throws IOException
   331{
   332// write out the *-Version header first, if it exists
   333String vername = Name.MANIFEST_VERSION.toString();
   334String version = getValue(vername);
   335if (version == null) {
   336vername = Name.SIGNATURE_VERSION.toString();
   337version = getValue(vername);
   338}
   339
   340if (version != null) {
   341out.writeBytes(vername+": "+version+"\r\n");
   342}
   343
   344// write out all attributes except for the version
   345// we wrote out earlier
   346for (Entry e : entrySet()) {
   347String name = ((Name) e.getKey()).toString();
   348if ((version != null) && 
!(name.equalsIgnoreCase(vername))) {


So, if there is no existing MANIFEST_VERSION or SIGNATURE_VERSION, 
then version is null and the check above will be false for ever and 
any other attribute cannot be written out.


Is this intended? If so, we can exit with an else block after line 342.


From code inspection, I agree that this method is a nop if there is no 
Manifest-Version attribute or Signature-Attribute.  This can return 
quickly without iterating the entry set.   I don't see any issue to 
make it an else block.


This method is only called from Manifest::write method which does not 
mention Signature-Version but I don't have the history to tell if this 
is intended or not.


I would assume the "intention" here is to do

if ((version != null) && name.equalsIgnoreCase(vername)))
continue;

sherman


Mandy

Re: RFR (JDK11) 8137326: Methods for comparing CharSequence, StringBuilder, and StringBuffer

[Tagir Valeev]

Hello!

An AbstractStringBuilder#compareTo implementation is wrong. You cannot
simply compare the whole byte array. Here's the test-case:

public class Test {
  public static void main(String[] args) {
StringBuilder sb1 = new StringBuilder("test1");
StringBuilder sb2 = new StringBuilder("test2");
sb1.setLength(4);
sb2.setLength(4);
System.out.println(sb1.compareTo(sb2));
System.out.println(sb1.toString().compareTo(sb2.toString()));
  }
}

We truncated the stringbuilders making their content equal, so
sb1.toString().compareTo(sb2.toString()) is 0, but compareTo compares
the original content (before the truncation) as truncation, of course,
does not zero the truncated bytes, neither does it reallocate the
array (unless explicitly asked via trimToSize).

With best regards,
Tagir Valeev.


On Fri, Jan 26, 2018 at 10:00 AM, Joe Wang  wrote:
> Hi,
>
> Adding methods for comparing CharSequence, StringBuilder, and StringBuffer.
>
> The Comparable implementations for StringBuilder/Buffer are similar to that
> of String, allowing comparison operations between two
> StringBuilders/Buffers, e.g. aStringBuilder.compareTo(anotherStringBuilder).
> For CharSequence however, refer to the comments in JIRA, a static method
> 'compare' is added instead of implementing the Comparable interface. This
> 'compare' method may take CharSequence implementations such as String,
> StringBuilder and StringBuffer, making it possible to perform comparison
> among them. The previous example for example is equivalent to
> CharSequence.compare(aStringBuilder, anotherStringBuilder).
>
> Tests for java.base have been independent from each other. The new tests are
> therefore created to have no dependency on each other or sharing any code.
>
> JBS: https://bugs.openjdk.java.net/browse/JDK-8137326
> webrev: http://cr.openjdk.java.net/~joehw/jdk11/8137326/webrev/
>
> Thanks,
> Joe