Re: RFR 8234147 : Avoid looking up standard charsets in core libraries

[Ulf Zibis]

Hi Ivan,

you may compare your proposal with:
https://bugs.java.com/bugdatabase/view_bug.do?bug_id=6790402
https://bugs.java.com/bugdatabase/view_bug.do?bug_id=6850361
https://bugs.java.com/bugdatabase/view_bug.do?bug_id=6796087

-Ulf

Am 27.11.19 um 05:39 schrieb Ivan Gerasimov:

Hello!

It is a cleanup fix with mostly two kinds of changes:
- when a standard charset is specified by its name, use a 
preinitialized Charset constant instead,
- replace the usage of StandardCharset.* constants with their 
sun.nio.cs.* equivalents to avoid accidental early initialization of 
rarely-used charsets.


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

I also had to modify one regression test that relied on a private 
auxiliary method, which was removed with the fix.


Mach5 control build looks green.

Would you please help review the fix?

Re: CharsetEncoder.maxBytesPerChar()

[Ulf Zibis]

Hey Martin,

great, that you got my issue. The link you shared is an interesting
basis for this discussion.

Maybe at some places e.g. in the "upfront specifications", additionally
the term "UTF-16 char" or "UTF-16 code unit" could be helpful and then
determining "char" or "{@code char}" as a short cut.

-Ulf

Am 27.09.19 um 15:04 schrieb Martin Buchholz:
> Like Ulf, I am sometimes annoyed by the use of the "character"
> misnomer throughout the API docs, and would support an effort to use
> "character" the way that unicode.org  uses it.
> "char" no longer represents a Unicode character, but at least it
> provides a short clear name, in the Java language, for "UTF-16 code
> unit" - if we use it consistently!
> https://unicode.org/faq/utf_bom.html#utf16-1
>
> On Thu, Sep 26, 2019 at 2:24 PM  > wrote:
>
> 2019/9/24 13:00:21 -0700, ulf.zi...@cosoco.de
> :
> > Am 21.09.19 um 00:03 schrieb mark.reinh...@oracle.com
> :
> >> To avoid this confusion, a more verbose specification might read:
> >>     * Returns the maximum number of $otype$s that will be
> produced for each
> >>     * $itype$ of input.  This value may be used to compute the
> worst-case size
> >>     * of the output buffer required for a given input sequence.
> This value
> >>     * accounts for any necessary content-independent prefix or
> suffix
> >> #if[encoder]
> >>     * $otype$s, such as byte-order marks.
> >> #end[encoder]
> >> #if[decoder]
> >>     * $otype$s.
> >> #end[decoder]
> >
> > wouldn't it be more clear to use "char" or even "{@code char}"
> instead
> > "character" as replacment for the $xtype$ parameters?
>
> The specifications of the Charset{De,En}coder classes make it clear
> up front that “character” means “sixteen-bit Unicode character,” so
> I don’t think changing “character” everywhere to “{@code char}” is
> necessary.
>
> This usage of “character” is common throughout the API specification.
> With the introduction of 32-bit Unicode characters we started calling
> those “code points,” but kept on calling sixteen-bit characters just
> “characters.”  (I don’t think the official term “Unicode code unit”
> ever caught on, and it’s a bit of a mouthful anyway.)
>
> - Mark
>

Re: CharsetEncoder.maxBytesPerChar()

[Ulf Zibis]

Am 21.09.19 um 00:03 schrieb mark.reinh...@oracle.com:
> To avoid this confusion, a more verbose specification might read:
>  * Returns the maximum number of $otype$s that will be produced for each
>  * $itype$ of input.  This value may be used to compute the worst-case 
> size
>  * of the output buffer required for a given input sequence. This value
>  * accounts for any necessary content-independent prefix or suffix
> #if[encoder]
>  * $otype$s, such as byte-order marks.
> #end[encoder]
> #if[decoder]
>  * $otype$s.
> #end[decoder]
>
> (The example of byte-order marks applies only to CharsetEncoders, so
>  I’ve conditionalized that text for Charset-X-Coder.java.template.)

Hi,

wouldn't it be more clear to use "char" or even "{@code char}" instead
"character" as replacment for the $xtype$ parameters?

-Ulf

Re: RFR: Some patches for sherman

[Ulf Zibis]

Fine enhancement!

See also:
JDK-6862158  : 
Make sun.nio.cs.* charset objects light-weight
JDK-6796087  : 
Speed-up sun.nio.cs.StandardCharsets
JDK-6790402  : 
Speed-up FastCharsetProvider


-Ulf


Am 27.03.2018 um 02:21 schrieb Martin Buchholz:

Motivation: I ran into trouble using reflection via
StandardCharsets. too early during bootstrap with a local mod.

: Avoid charset lookup machinery in StandardCharsets
http://cr.openjdk.java.net/~martin/webrevs/jdk/StandardCharsets/

Re: RFR: 8197594 - String and character repeat

[Ulf Zibis]

Am 18.02.2018 um 06:10 schrieb Stuart Marks:

Fair enough. I'll be less unhappy if there is a way to convert from a code 
point to a String, as requested by JDK-4993841. This will reduce

 new String(Character.toChars(codepoint)).repeat(count)

to

 Character.toString(codepoint).repeat(count)
Shorter and maybe more logical to get a String by a String constructor, 
instead overloading toString() of Character:

    String(codepoint).repeat(count)

-Ulf

Re: RFR: JDK-8021560,(str) String constructors that take ByteBuffer

[Ulf Zibis]

Hi,

what is the difference of:
- class StringCoding
- class StringCoder
- class StringCoding.StringDecoder
- class StringCoding.StringEncoder

Why we need so much variants of redundant code? I think it would be 
useful to have some explanation in the javadoc.


Missing indentation in StringCoder.java :

 322 if (enc == null) {
 323 enc = cs.newEncoder()
 324 .onMalformedInput(CodingErrorAction.REPLACE)
 325 .onUnmappableCharacter(CodingErrorAction.REPLACE);
 326 }


-Ulf


Am 16.02.2018 um 00:25 schrieb Xueming Shen:


http://cr.openjdk.java.net/~sherman/8021560/webrev.v2

-Sherman

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

[Ulf Zibis]

Hi Claes,

sorry for the delay, I didn't catch you message before, because you CC'd 
me which causes that my email filter misses the ListID tag to catch.


Thanks for the update, times change with the compact strings feature.

Another approach could be to access the String constant via Unsafe.

-Ulf


Am 30.01.2018 um 11:12 schrieb Claes Redestad:

Hi,

not sure what you're suggesting, exactly, but it seems Charset coders 
generate
String literals which are then unpacked to char[]/byte[]'s in static 
blocks.


While constructing arrays from String literals might have been a startup
optimization in the past, it is now mainly a workaround to method 
bytecode

limits, see https://bugs.openjdk.java.net/browse/JDK-8185104

/Claes

On 2018-01-30 01:10, Ulf Zibis wrote:

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: 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: RFR JDK-8186751: Add ISO-8859-16 Charset support

[Ulf Zibis]

Hi again,


Am 31.08.2017 um 23:29 schrieb Xueming Shen:

The 8859-16 aliases are the copy/paste from
https://www.iana.org/assignments/character-sets/character-sets.xhtml

From the same page we have the followings for 8859-15
ISO_8859-15
Latin-9
csISO885915

It appears only the first one is listed in our alias list (explicitly 
commented).

Don't have the memory for the history (8859-15 was added a long time ago),
not sure whether the other two were missed at the very beginning or were
added later in the iana. Since we are here, I added them in.


Fine!


http://cr.openjdk.java.net/~sherman/8186751/webrev
(the old one is copied to webrev.00)


- I think, the comment should now be in plural: # IANA alias*es
*- Also I think, it would make sense to at least add "ISO8859-16" and 
"ISO8859_16" as # Other aliases
- "ISO-8859-15" must not be listed as alias, as it already is the 
canonical name.

- Not sure, where "LATIN0" and "csISOlatin0" come from.

-Ulf

Re: [10] RFR: 8185104: Generate CharacterDataLatin1 lookup tables directly

[Ulf Zibis]

Hi,


Am 23.07.2017 um 15:37 schrieb Claes Redestad:
1. it reduces the size of the generated bytecode, which is necessary 
to keep code below method bytecode limits if the arrays generated are 
very large


I always was wondering why filling large lookup tables is done by static 
java code in the class. Wouldn't it be more clever, to load the lookup 
table content from a binary resource file? Then we must not bother to 
initialize tables by static fake strings to save bytecode footprint.
The same thoughts would apply to the charset mappings in sun.nio.charset 
classes.


-Ulf

Re: [NEW BUG]: Avoid allocations in String.replace(CharSequence, CharSequence) in case no replacement happened

[Ulf Zibis]

Am 02.03.2017 um 18:16 schrieb Vitaly Davidovich:
When the optimization applies, there shouldn't be *any* instructions for the null check on the 
path - it's done via signal handling.

Thanks for clarification.

This is Sufficiently Smart Compiler territory.  It's much better to hand-code this properly, 
particularly since nothing is lost in readability or maintainability in this case.

Convinced!

-Ulf

Re: [NEW BUG]: Avoid allocations in String.replace(CharSequence, CharSequence) in case no replacement happened

[Ulf Zibis]

Am 02.03.2017 um 14:55 schrieb Vitaly Davidovich:

Implicit null checks ... generally don't cost anything.

Any additional byte of code in CPU cache prevents other code from staying there.



I can't imagine, if any programmer would rely on such contract, instead of 
doing the check
explicitly if of significance.

I think it's to have consistent behavior, irrespective of control flow.  It can 
detect bugs early on.
Yes, serving bug detection almost always has some cost and should be pondered, but the issue of this 
patch is to increase performance. Here I suspect the real value of the bug detection help.



I think, the original reasonable of the contract is to rule out any other 
exception type or
undefined behaviour in case of relevance of the argument. So I would vote 
for stating the contract
more precisely in that way.

Also, if I remember correct, if a variable, here replStr, is only 
referenced later, the
compiler can
move the execution of here replacement.toString() to a later position, so 
it is not clear to
me, if
the original implementation ever fulfilled the contract.

Compiler can only do that if it proves a bunch of things, such as no observable side effects as a 
result of sinking it down.  If CharSequence.toString isn't devirtualized there, eg, it definitely 
cannot do it.  I wouldn't bet on it.
For the original code in case of inlining, I guess, the optimizer would place the null check before 
the if block and the allocation of the replacement String after. For the new code, the optimizer can 
detect the duplicate null check inside and outside the if block, so can move it before the if block. 
So both java codes could result in the same machine code. IMHO the effective optimization should be 
proved by HS disassembler.


There is also the possibility, that an allocation of an intermediate String object is never done, 
because the anyway involved StringBuilder perhaps can insert the replacement sequence without before 
instantiating a String object.


-Ulf

Re: [NEW BUG]: Avoid allocations in String.replace(CharSequence, CharSequence) in case no replacement happened

[Ulf Zibis]

HI,

first sorry for missing the class wide definition.

Am 02.03.2017 um 12:45 schrieb Vitaly Davidovich:


On Thu, Mar 2, 2017 at 6:38 AM Ulf Zibis <ulf.zi...@cosoco.de 
<mailto:ulf.zi...@cosoco.de>> wrote:

In any case, what's the reasonable of checking an argument, which is not 
used in that case?

My understanding is contracts like the above (let's assume it still called out the NPE in 8) are 
checked even if the argument isn't used in some cases.  The other case where I believe this is 
done is when passing a Supplier to some method that uses it to obtain a default value - even if 
it's not needed, it's checked for null because most (all?) such methods stipulate that it cannot 
be null.
On the other hand, the null check is a waste of performance and footprint (implicits performance 
cost too).
I can't imagine, if any programmer would rely on such contract, instead of doing the check 
explicitly if of significance.


I think, the original reasonable of the contract is to rule out any other exception type or 
undefined behaviour in case of relevance of the argument. So I would vote for stating the contract 
more precisely in that way.


Also, if I remember correct, if a variable, here replStr, is only referenced later, the compiler can 
move the execution of here replacement.toString() to a later position, so it is not clear to me, if 
the original implementation ever fulfilled the contract.


-Ulf

Re: [NEW BUG]: Avoid allocations in String.replace(CharSequence, CharSequence) in case no replacement happened

[Ulf Zibis]

Hi Vitaly,

I don't see any contract to throw an NPE:
https://docs.oracle.com/javase/8/docs/api/java/lang/String.html#replace-java.lang.CharSequence-java.lang.CharSequence-

In any case, what's the reasonable of checking an argument, which is not used 
in that case?


Am 02.03.2017 um 00:18 schrieb Vitaly Davidovich:

Seems like a good idea.  You probably want to null check 'replacement'
before the bail out as the method is specified as throwing NPE in that case.

Re: [9] RFR 8170769: Provide a simple hexdump facility for binary data

[Ulf Zibis]

Hi,

I would prefer a "normal" class instead a convolut of static methods. Via a normal constructor, we 
could pass some custom parameters e.g. capital/uppercase letters for "abcdef", prefix a header line, 
width of the index counter, bytes per line, i.e. have all the parameters, you have hardcoded, variable.


Additionally I would like to see a method with variable start and end:

String dump(byte[] bytes, int start, int length)


-Ulf

Am 07.12.2016 um 17:32 schrieb Vincent Ryan:

A hexdump facility has been available for many, many years via an unsupported 
class: sun.misc.HexDumpEncoder.
Although that class was always unsupported, it was still accessible. That 
accessibility changes with Jigsaw so I’m proposing
a very simple replacement in a new and supported class: java.util.HexDump.

Thanks.

Bug: https://bugs.openjdk.java.net/browse/JDK-8170769
Webrev: http://cr.openjdk.java.net/~vinnie/8170769/webrev.00/

Re: RFR 8170348: Appendable.appendN(char, int) method to append multiple copies of char

[Ulf Zibis]

Am 08.12.2016 um 09:28 schrieb Peter Levart:


On 12/07/2016 11:28 PM, Roger Riggs wrote:

AbstractStringBuilder:
   I agree with Claes' comment suggesting that IAE for negative lengths is a 
pain
   and defining it to append 0 would be natural in many use cases. 


OTOH, inserting a simple Math.max(n, 0) instead of n where n could get negative would achieve the 
same without complicating the expression too much. Java standard APIs have a tradition of being 
explicit rather than having implicit hidden logic which surely shortens many usecases, but makes 
them harder to read and understand for casual readers not intimately familiar with such API. The 
logic to treat negative lengths as 0 is implicit and not universally correct.

+1
If we would treat negative values as 0, we loose a chance, where programmers could become aware 
about possible errors in the logic of their program.


-Ulf

Re: RFC: System.console().encoding()

[Ulf Zibis]

Am 15.09.2016 um 17:56 schrieb Aleksey Shipilev:


...which opens a way to poll this without a Reflection hack. Extended
the JMH hack with it, but it still fragile:
  http://hg.openjdk.java.net/code-tools/jmh/rev/8c20adb08b2d


Maybe keep it simple - no need for (prop != null) - and in design line with the 
2 other tries:

// Try 3. Try to poll internal properties.
try {
return Charset.forName(System.getProperty("sun.stdout.encoding") );
} catch (Exception e) {
// fall-through
}


-Ulf

Re: RFR: JDK-8061777, , (zipfs) IllegalArgumentException in ZipCoder.toString when using Shitft_JIS

[Ulf Zibis]

Am 05.06.2016 um 21:30 schrieb Ulf Zibis:
And I still think, moving the string concatenation to ZipPath constructor would be a good idea. I 
believe, this would make things more simple and less redundant:

ZipPath(ZipFileSystem zfs, String first, String... more)
E.g. you could reuse the StringBuilder in directly passing it to normalize() and save the 
transformation to String.


-Ulf

Re: RFR: JDK-8061777, , (zipfs) IllegalArgumentException in ZipCoder.toString when using Shitft_JIS

[Ulf Zibis]

Am 05.06.2016 um 21:30 schrieb Ulf Zibis:

Is this the answer to my # 3.) ?
I really often see Windows-1252 coded zip files for download in the web. Maybe you see them rarely 
as american, but if there are german Umlaute in the file names, it' really painful to convert all 
the paths here on UTF-8 Linux.
To be honest, in my early java times I used german Umlaute for class names and I developed on 
Windows, and as I had to update such a project later, I was really wondering, that the compiled 
jar's didn't work on Linux. So the use case can occur on jar's too.


-Ulf

Re: RFR: JDK-8061777, , (zipfs) IllegalArgumentException in ZipCoder.toString when using Shitft_JIS

[Ulf Zibis]

Sherman, thanks for your consideration.

Am 31.05.2016 um 19:08 schrieb Xueming Shen:

Ulf, thanks for the suggestions!

On 5/31/16 6:27 AM, Ulf Zibis wrote:

Hi Sherman,

1.) wouldn't it be better to have a public getBytes() in AbstractStringBuilder?
Then you could save the array copy with sb.toString() here:


One single use case here probably is not a strong enough reason to
add such a utility method into ASB.
Yes, not for a single use case. But I was wondering, that such method is not part of StringBuilder 
since long, as I could think about many use cases.



I was hesitated to exposure zfs.zc to be package private for some reason, that
was why have the pair of zfs.getBytes/String(). But sure I can do it if it makes
the communication clearer.

Yes fine, and it saves method parameters to be passed.
I'm also wondering, why ZipCoder isn't a sub class of j.n.c.Charset


6.) Avoid String instatiation especially when called from child paths iterator 
*loop*.


This can be a candidate for further optimization.

Yes.


Currently my assumption is that
non-utf8 jar/zip files are not the main use cases for zipfs (assume most of them
are the result of either the jar tool or j.u.zip apis),

Is this the answer to my # 3.) ?
I really often see Windows-1252 coded zip files for download in the web. Maybe you see them rarely 
as american, but if there are german Umlaute in the file names, it' really painful to convert all 
the paths here on UTF-8 Linux.


And I still think, moving the string concatenation to ZipPath constructor would be a good idea. I 
believe, this would make things more simple and less redundant:

ZipPath(ZipFileSystem zfs, String first, String... more)

Thanks,
-Ulf

Re: RFR: JDK-8061777, , (zipfs) IllegalArgumentException in ZipCoder.toString when using Shitft_JIS

[Ulf Zibis]

Hi Sherman,

1.) wouldn't it be better to have a public getBytes() in AbstractStringBuilder?
Then you could save the array copy with sb.toString() here:
 178 return new ZipPath(this, sb.toString(), zc.isUTF8());
 525 return zfs.getBytes(to.toString());


You could simplify even more.

2.) No need for fork on isUTF8 in ZFS.iteratorOf() and for parameter isUTF8:
  55 ZipPath(ZipFileSystem zfs, byte[] path, boolean normalized)
  56 {
  57 this.zfs = zfs;
  58 if (normalized)
  59 this.path = path;
  60 // if zfs is NOT in utf8, normalize the path as "String"
  61 // to avoid incorrectly normalizing byte '0x5c' (as '\')
  62 // to '/'.
  63 else if (zfs.zc.isUTF8())
  64 this.path = normalize(path);
  65 else
  66 this.path = normalize(zfs.getString(path));
  67 }

  64 ZipPath(ZipFileSystem zfs, String path) {
  65 this.zfs = zfs;
  66 // if zfs is NOT in utf8, normalize the path as "String"
  67 // to avoid incorrectly normalizing byte '0x5c' (as '\')
  68 // to '/'.
  69 if (zfs.zc.isUTF8()) {
  70 this.path = normalize(zfs.getBytes(path));
  71 } else {
  72 this.path = normalize(path);
  73 }
  74 }
  75

3.) We could benefit from better byte array performance for all one byte charsets, e.g. common 
Windows-1252:

With:
if (zfs.zc.isOneByte())
this.path = normalize(path, zfs.zc.backSlashCode, zfs.zc.slashCode);

4.) We could do the same for all double byte charsets and benefit from the new intrinsic accessor 
methods on byte arrays/buffers with VarHandles.


5.) As alternative to 2.) consider moving the string concatenation to ZipPath constructor. I 
believe, this would make things more simple and less redundant:

ZipPath(ZipFileSystem zfs, String first, String... more)

6.) Avoid String instatiation especially when called from child paths iterator 
*loop*.
Instead:
 500 if (len > 1 && prevC == '/')
 501 path = path.substring(0, len - 1);
 502 return zfs.getBytes(path);
provide:
 500 return zfs.getBytes(path, 0, len - (len > 1 && prevC == '/' ? 1 : 
0 ));

7.) Last but not least, the normalize algorithm could be an additional usecase 
for
http://bugs.java.com/bugdatabase/view_bug.do?bug_id=6695386
with '\' as terminator.

-Ulf


Am 31.05.2016 um 07:07 schrieb Xueming Shen:

Thanks Paul,

updated accordingly.

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

-sherman

On 5/30/16 2:31 AM, Paul Sandoz wrote:

Hi Sherman,

Do you consider modifying the new ZipPath constructor you added to accept a boolean value for 
UTF-8 encoding?


If so you can more clearly document the behaviour and avoid duplication of the operators in 
ZipFileSystem e.g.:


   return new ZipPath(this, first, zc.isUTF8());

Paul.

Re: RFR: 8155795: Optimize Integer/Long.reverse by using reverseBytes

[Ulf Zibis]

Am 02.05.2016 um 17:00 schrieb Claes Redestad:

The reverseBytes changes were motivated by seeing slightly better
performance on the micro with the suggested changes, but after
discussing this a bit I think we should revert to the original code alone
and investigate if there's some compiler quirk lurking here separately.
Maybe (i & 0xFF00) is faster than (i & 0xFF), because the first can be executed by shorter 
16-bit CPU op code.

Looking at HotSpot disassembler output could solve the miracle.

-Ulf

Re: RFR: 8154231: Simplify access to System properties from JDK code

[Ulf Zibis]

Hi Claes, thanks.

Am 20.04.2016 um 18:12 schrieb Claes Redestad:

Thanks for looking at this, Ulf!

- Isn't the "theProp" naming style something from an old usage where all members have been named 
myXyz? I more would like "propName" or just "property".


I chose to go with keeping names in line with existing methods. If anything is to be done about 
this I'd prefer changing all names in the class to be consistent and modern, and that'd be a 
separate RFE.


Oops, I didn't notice that.
... and semantically more precise would be: "propKey" or "propID".

-Ulf

Re: RFR: 8154231: Simplify access to System properties from JDK code

[Ulf Zibis]

Hi,

here my comments:

Am 20.04.2016 um 16:44 schrieb Claes Redestad:

Hello,

now that the sun.security.action package is encapsulated we can simplify internal code to get 
System properties.


Bug: https://bugs.openjdk.java.net/browse/JDK-8154231
Webrev: http://cr.openjdk.java.net/~redestad/8154231/webrev.01/

This adds a few convenience methods to GetPropertyAction[1] and 
GetIntegerAction[2].


- In the original version of ProcessBuilder.java line 472 there was a nice example using lambda 
expression. Wouldn't this be applicable also for these new methods here?
- Isn't the "theProp" naming style something from an old usage where all members have been named 
myXyz? I more would like "propName" or just "property".
- In Integer.getProperty the default substitution is done twice, lines: 143, 148. So maybe return to 
the "immediate return in if...else clause" style.


-Ulf

Re: RFR: 8145680: Remove unnecessary explicit initialization of volatile variables in java.base

[Ulf Zibis]

Am 21.12.2015 um 11:31 schrieb Ulf:

Am 21.12.2015 um 00:06 schrieb Claes Redestad:

Hi all,

On 2015-12-20 20:43, Ulf wrote:

Hi Claes,

I had a very short look and found in j.l.Thread:
 211  * Java thread status for tools, 0 indicate thread 'not yet started'
I guess you meant:
 211  * Java thread status for tools; 0 indicates: thread 'not yet started'.

-Ulf 


/*
 * Java thread status for tools, default indicates thread 'not yet started'
 */

OK?


To my feeling, 0 is more clear than default.
Often enough the default for a value is other than 0, so the user ends up to search for "What is 
the default of this value?".
Also I think, the expression "thread 'not yet started'" should be separated in some way from the 
rest of the phrase, maybe just shift the "'" one word before.


Using both seems a good compromise, and remember punctuation please:
 * Java thread status for tools(,|;) default 0 indicates: thread 'not yet 
started'.

-Ulf

Re: RFR (S) 8145428: Optimize StringUTF16 compress/copy methods for C1

[Ulf Zibis]

Am 17.12.2015 um 08:54 schrieb Aleksey Shipilev:

On 12/17/2015 02:34 AM, Ulf wrote:

I'm wondering why moving the increment operation to an extra line wound
enhance performance.

Because C1 is very straightforward, and code movement like that is a
poor man's instruction scheduling, that pads out the data dependency
between index update and indexed access.


Thanks.
Shouldn't this simple optimization be addressed to javac?
Otherwise programmers always risk some performance when using the post-increment idiom, which IMHO 
is good code style.


-Ulf

Re: Suggested fix for JDK-4724038 (Add unmap method to MappedByteBuffer)

[Ulf Zibis]

Am 08.09.2015 um 20:37 schrieb Andrew Haley:
I think that MR is referring to Windows when he talks about race conditions. Andrew. 


Couldn't we introduce a unmap() method which throws an UnsupportedOperationException if the 
underlying OS isn't able to unmap the the buffer safely in the mean time?


-Ulf

Re: RFR 8124977 cmdline encoding challenges on Windows

[Ulf Zibis]

Am 08.07.2015 um 18:50 schrieb Kirk Shoop:

Valery prepared a new webrev which I placed here:
   http://cr.openjdk.java.net/~kshoop/8124977/webrev.01


May be better to avoid potetially superfluous lookup of UTF-8:

public static Charset defaultUnicodeCharset() {
if (defaultUnicodeCharset == null) {
synchronized (Charset.class) {
String csn = AccessController.doPrivileged(
new GetPropertyAction(file.encoding.unicode));  // indent 
8 spaces !!
if (csn == null || (defaultUnicodeCharset =lookup(csn)  == null)) {
defaultUnicodeCharset = forName(UTF-8);
}
}
}
return defaultUnicodeCharset;
}


-Ulf

Re: String.contains(CharSequence) calls toString on argument

[Ulf Zibis]

Hi,

thanks for your feedback.
I think it would be more readable here, if you would use if ... else ... .
To me a ternary operator is more readable, if there is one value to compute. Also it enhances 
readability over the whole code, if I use less lines as possible, in other words, I hate to scroll 
so much.


-Ulf


Am 28.05.2015 um 12:57 schrieb Tomasz Kowalczewski:

Hi,

thank you for taking time to read my proposal. Is this a matter of some OpenJDK coding 
conventions? I myself never use ternary operator and always use if with braces. My experience is 
that it improves code readability. If I have to apply one of proposed changes I would choose first 
or third.


Regards,
Tomasz

On Thu, May 28, 2015 at 12:35 PM, Ulf Zibis ulf.zi...@cosoco.de 
mailto:ulf.zi...@cosoco.de wrote:

Hi,

I more would like:

2199 return (s instanceof String)
2200 ? indexOf((String) s) = 0;
2201 : indexOf(value, 0, value.length, s, 0, s.length(), 0) 
= 0;

or:

2199 return (s instanceof String
2200 ? indexOf((String) s)
2201 : indexOf(value, 0, value.length, s, 0, s.length(), 0))
2202 = 0;

or:

2199 int index = (s instanceof String)
2200 ? indexOf((String) s)
2201 : indexOf(value, 0, value.length, s, 0, s.length(), 0);
2202return  index  = 0;

-Ulf



Am 28.05.2015 um 12:06 schrieb Tomasz Kowalczewski:

Hello all,

encouraged by your responses I did a simple implementation and performed
some JMH experiments. Details below.

Webrev [1] contains my changes. I deliberately wanted to do minimal code
changes. The gist of it is implementing indexOf that works on 
CharSequence
instead of String's internal char array. Both versions are almost 
identical
(I did not change existing implementation in place to not impact all 
other
String-only paths that use it).

In my first attempt I just inlined (and simplified) indexOf 
implementation
into String::contains, but in the end decided to create general purpose
(package private) version. This might prove useful for others[2]

JMH test project is here [3]. All benchmarks were run using java -jar
benchmarks.jar -wi 10 -i 10 -f -prof gc on Amazon EC2 instance 
(c4.xlarge,
4 virtual cores). I have run it against three java builds:

a. stock java 8u40
b. my own build from clean jdk9 sources
c. my own build from modified jdk9 sources

Results with gc profiler enabled are included in benchmark repo. This 
gist
contains summary results [4].

I know that this test covers only very narrow space of possible 
performance
regressions (ideas form more comprehensive tests are welcome). The 
results
show that String only path is not impacted by my changes. When using 
actual
CharSequence (StringBuilder in this case) we are mostly winning, 
exception
is case when CharSequence is of medium size and has many partial 
matches in
searched string.

I hope this exercise will be useful to someone and that this change 
might
be considered for inclusion in OpenJDK. If not then maybe at least tests
for String::indexOf?

[1] http://openjdk.s3-website-us-east-1.amazonaws.com/
[2] Changeset from

http://mail.openjdk.java.net/pipermail/core-libs-dev/2015-May/033678.html uses
indexOf that could accept CharSequence instead of String.
[3] https://github.com/tkowalcz/openjdk-jmh
[4] https://gist.github.com/tkowalcz/d8f5b9435e184f65fd2a

Regards,
Tomasz Kowalczewski

On Sat, Mar 21, 2015 at 9:21 AM, Tomasz Kowalczewski 
tomasz.kowalczew...@gmail.com mailto:tomasz.kowalczew...@gmail.com 
wrote:

There are other places which accept CharSequence and iterate over it
not caring about possible concurrent modification (the only sane way
IMHO). Examples are String.contentEquals and String.join that uses
StringJoiner which iterates its argument char by char.

It looks like contains is the exception in this regard.

On 20 mar 2015, at 17:05, Vitaly Davidovich vita...@gmail.com
mailto:vita...@gmail.com wrote:

If CharSequence is mutable and thread-safe, I would expect 
toString()
implementation to provide the atomic snapshot (e.g. do the
synchronization). Therefore, I find Sherman's argument 
interesting.


That's my point about it ought to be up to the caller -- 
they're in a
position to make this call, but String.contains() is playing 
defensive
because it has no clue of the context.

On Fri, Mar 20, 2015 at 11:55 AM, Aleksey

Re: 8058779: Faster implementation of String.replace(CharSequence, CharSequence)

[Ulf Zibis]

Hi,

Am 31.05.2015 um 18:03 schrieb Ivan Gerasimov:


On 31.05.2015 18:54, Ivan Gerasimov wrote:


I fixed the code and added the case to the regression test in the new webrev.


Which is right here:
http://cr.openjdk.java.net/~igerasim/8058779/05/webrev/


Shoudn't the user be informed via javadoc about the risk of an OutOfMemoryError, just from illegal 
arguments of this method?

Example:
this.value.length() = 100
target.value.length() = 200
replacement.value.length() = 50
results in:
newLenHint = -500 -- OutOfMemoryError
In other words, is this a good reason to throw such an Error?

Little nit:
Indentation for line continuation should be 8 spaces.

-Ulf

Re: 8058779: Faster implementation of String.replace(CharSequence, CharSequence)

[Ulf Zibis]

Am 31.05.2015 um 08:38 schrieb Peter Levart:

Hi,

Yes, this one is much easier to grasp.

As I understand the check is to avoid overflow in calculation of StringBuilder initial capacity 
(newLenHint). If overflow happened, newLenHint would be negative and StringBuilder construction 
would fail with NegativeArraySizeException. But the calculation of newLenHint:


int newLenHint = value.length - targLen + replValue.length;

...considering the following:

value.length = 0
targLength = 0
replValue.length = 0
targLength = value.length

in case of overflow, it can only produce a negative value. So the check could 
simply be:

if (newLenHint  0) {
throw new OutOfMemoryError();
}


Hm, what has this situation to do with Out-Of-Memory ?

In other words, IMHO NegativeArraySizeException is much better here and 
additionally saves performance.
Additionally you could propagate it to InvalidArgumentException.

-Ulf

Re: RFR JDK-8081452: Move sun.nio.cs.AbstractCharsetProvider into jdk.charset/sun.nio.cs.ext

[Ulf Zibis]

Am 30.05.2015 um 03:26 schrieb Xueming Shen:

On 5/29/15 4:02 PM, Ulf Zibis wrote:


Am 29.05.2015 um 19:42 schrieb Xueming Shen:
But if it is decided later that we may want to have a separate ext charsets provider2, for 
example to split most of the ibm charsets to a separate provider, it might be desired to keep it 
as a base class ...

Hm, is it mandatory, that each charset provider must have it's own class?
I also think, that we do not need a separate class for each charset.
No, it's not a must to have a separate class for each charset, but it's a logical way to 
organize those
charset with their data. Given how most of these charsets are src-generated in current jdk, it's 
fair easy
to actually generate a charset object from a base classes (SingleByte, or DoubleByte) + a set of 
data

( such as the name, aliases table, mapping data, etc) during runtime, without 
having a real concrete
charset class. But then you need to figure out a better way to organize, store and 
read/initialize  those

data in a optimized way to initialize each charset on the fly, which we are 
now utilizing the jvm's
class initialization mechanism to achieve this. Any benefit/advantage of doing 
this? We might throw
in some resource someday to gather some data ...


Hi sherman,

I did some work here:
https://bugs.openjdk.java.net/show_bug.cgi?id=100090
https://bugs.openjdk.java.net/show_bug.cgi?id=100091
https://bugs.openjdk.java.net/show_bug.cgi?id=100092
https://bugs.openjdk.java.net/show_bug.cgi?id=100095
https://bugs.openjdk.java.net/show_bug.cgi?id=100098
https://bugs.openjdk.java.net/show_bug.cgi?id=1000104
https://bugs.openjdk.java.net/show_bug.cgi?id=1000105
https://bugs.openjdk.java.net/show_bug.cgi?id=1000107
https://bugs.openjdk.java.net/show_bug.cgi?id=1000132
Unfortunately the data was moved somewhere. Do you know, where the data was 
moved?
One of the original bug reports: 
http://bugs.java.com/bugdatabase/view_bug.do?bug_id=6862158

Also I did some work here:
https://java.net/projects/java-nio-charset-enhanced

-Ulf

Re: Why isn't Object.notify() a synchronized method?

[Ulf Zibis]

Thanks for your hint David. That's the only reason I could imagine too.
Can somebody tell something about the cost for recursive lock acquisition in comparison to the whole 
call, couldn't it be eliminated by Hotspot?


As I recently fell into the trap of forgetting the synchronized block around a single notifyAll(), I 
believe, the current situation is just errorprone.


Any comments about the Javadoc issue?

-Ulf


Am 28.05.2015 um 18:27 schrieb David M. Lloyd:
Since most of the time you have to hold the lock anyway for other reasons, I think this would 
generally be an unwelcome change since I expect the cost of recursive lock acquisition is nonzero.


On 05/28/2015 11:08 AM, Ulf Zibis wrote:

Hi all,

in the Javadoc of notify(), notifyAll() and wait(...) I read, that this
methods should only be used with synchronisation on it's instance.
So I'm wondering, why they don't have the synchronized modifier out of
the box in Object class.

Also I think, the following note should be moved from wait(long,int) to
wait(long):
/The current thread must own this object's monitor. The thread releases
ownership of this monitor and waits until either of the following two
conditions has occurred://
/

  * /Another thread notifies threads waiting on this object's monitor to
wake up either through a
call to the notify method or the notifyAll method./
  * /The timeout period, specified by timeout milliseconds plus nanos
nanoseconds arguments, has
elapsed. /



Cheers,

Ulf

Re: RFR JDK-8081452: Move sun.nio.cs.AbstractCharsetProvider into jdk.charset/sun.nio.cs.ext

[Ulf Zibis]

Am 29.05.2015 um 19:42 schrieb Xueming Shen:
But if it is decided later that we may want to have a separate ext charsets provider2, for example 
to split most of the ibm charsets to a separate provider, it might be desired to keep it as a base 
class ...

Hm, is it mandatory, that each charset provider must have it's own class?
I also think, that we do not need a separate class for each charset.

-Ulf

Re: String.contains(CharSequence) calls toString on argument

[Ulf Zibis]

Hi,

I more would like:

2199 return (s instanceof String)
2200 ? indexOf((String) s) = 0;
2201 : indexOf(value, 0, value.length, s, 0, s.length(), 0) = 
0;

or:

2199 return (s instanceof String
2200 ? indexOf((String) s)
2201 : indexOf(value, 0, value.length, s, 0, s.length(), 0))
2202 = 0;

or:

2199 int index = (s instanceof String)
2200 ? indexOf((String) s)
2201 : indexOf(value, 0, value.length, s, 0, s.length(), 0);
2202return  index  = 0;
 


-Ulf


Am 28.05.2015 um 12:06 schrieb Tomasz Kowalczewski:

Hello all,

encouraged by your responses I did a simple implementation and performed
some JMH experiments. Details below.

Webrev [1] contains my changes. I deliberately wanted to do minimal code
changes. The gist of it is implementing indexOf that works on CharSequence
instead of String's internal char array. Both versions are almost identical
(I did not change existing implementation in place to not impact all other
String-only paths that use it).

In my first attempt I just inlined (and simplified) indexOf implementation
into String::contains, but in the end decided to create general purpose
(package private) version. This might prove useful for others[2]

JMH test project is here [3]. All benchmarks were run using java -jar
benchmarks.jar -wi 10 -i 10 -f -prof gc on Amazon EC2 instance (c4.xlarge,
4 virtual cores). I have run it against three java builds:

a. stock java 8u40
b. my own build from clean jdk9 sources
c. my own build from modified jdk9 sources

Results with gc profiler enabled are included in benchmark repo. This gist
contains summary results [4].

I know that this test covers only very narrow space of possible performance
regressions (ideas form more comprehensive tests are welcome). The results
show that String only path is not impacted by my changes. When using actual
CharSequence (StringBuilder in this case) we are mostly winning, exception
is case when CharSequence is of medium size and has many partial matches in
searched string.

I hope this exercise will be useful to someone and that this change might
be considered for inclusion in OpenJDK. If not then maybe at least tests
for String::indexOf?

[1] http://openjdk.s3-website-us-east-1.amazonaws.com/
[2] Changeset from
http://mail.openjdk.java.net/pipermail/core-libs-dev/2015-May/033678.html uses
indexOf that could accept CharSequence instead of String.
[3] https://github.com/tkowalcz/openjdk-jmh
[4] https://gist.github.com/tkowalcz/d8f5b9435e184f65fd2a

Regards,
Tomasz Kowalczewski

On Sat, Mar 21, 2015 at 9:21 AM, Tomasz Kowalczewski 
tomasz.kowalczew...@gmail.com wrote:


There are other places which accept CharSequence and iterate over it
not caring about possible concurrent modification (the only sane way
IMHO). Examples are String.contentEquals and String.join that uses
StringJoiner which iterates its argument char by char.

It looks like contains is the exception in this regard.

On 20 mar 2015, at 17:05, Vitaly Davidovich vita...@gmail.com wrote:


If CharSequence is mutable and thread-safe, I would expect toString()
implementation to provide the atomic snapshot (e.g. do the
synchronization). Therefore, I find Sherman's argument interesting.


That's my point about it ought to be up to the caller -- they're in a
position to make this call, but String.contains() is playing defensive
because it has no clue of the context.

On Fri, Mar 20, 2015 at 11:55 AM, Aleksey Shipilev 
aleksey.shipi...@oracle.com wrote:


If CharSequence is mutable and thread-safe, I would expect toString()
implementation to provide the atomic snapshot (e.g. do the
synchronization). Therefore, I find Sherman's argument interesting.

On the other hand, we don't seem to be having any details/requirements
for contains(CharSequence) w.r.t. it's own argument. What if
String.contains pulls the values one charAt at a time? The concurrency
requirements are not enforceable in CharSequence alone then, unless you
call the supposed-to-be-atomic toString() first.

-Aleksey.


On 03/20/2015 06:48 PM, Vitaly Davidovich wrote:
Yes, but that ought to be for the caller to decide.  Also, although the
resulting String is immutable, toString() itself may observe mutation.

On Fri, Mar 20, 2015 at 11:40 AM, Xueming Shen 

xueming.s...@oracle.com

wrote:


On 03/20/2015 02:34 AM, Tomasz Kowalczewski wrote:

Hello!

Current implementation of String.contains that accepts CharSequence

calls

toString on it and passes resulting string to indexOf(String). This

IMO

defeats the purpose of using CharSequences (that is to have a mutable
character buffer and not allocate unnecessary objects).

It is arguable that cs.toString() may serve the purpose of taking a
snapshot of an otherwise
mutable character buffer?

-Sherman


Is changing this a desirable development? It seems pretty

straightforward

to port indexOf(String) to use CharSequence.

If 

Why isn't Object.notify() a synchronized method?

[Ulf Zibis]

Hi all,

in the Javadoc of notify(), notifyAll() and wait(...) I read, that this methods should only be used 
with synchronisation on it's instance.

So I'm wondering, why they don't have the synchronized modifier out of the box 
in Object class.

Also I think, the following note should be moved from wait(long,int) to 
wait(long):
/The current thread must own this object's monitor. The thread releases ownership of this monitor 
and waits until either of the following two conditions has occurred://

/

 * /Another thread notifies threads waiting on this object's monitor to wake up 
either through a
   call to the notify method or the notifyAll method./
 * /The timeout period, specified by timeout milliseconds plus nanos 
nanoseconds arguments, has
   elapsed. /



Cheers,

Ulf

Re: RFR (XS) 8076759: AbstractStringBuilder.append(...) should ensure enough capacity for the upcoming trivial append calls

[Ulf Zibis]

Am 08.05.2015 um 13:28 schrieb Aleksey Shipilev:

On 05/07/2015 06:00 PM, Ulf Zibis wrote:

May be:
..., then a new internal array becomes allocated and refilled with
greater capacity.

... to give a hint, that this action may be expensive.

The Javadoc already says If the current capacity is less than the
argument, then a new internal array is allocated with greater capacity,
which seems to be what you are saying.


Allocating a new array is one thing, but copying the old data into the new one is another story 
where the cost is dependent on it's size. Also theoretically, AbstractStringBuilder could hold 
multiple arrays to avoid the copying and solve memory fragmentation issues. Both aspects are 
implementation details, so when mentioned in Javadoc, my thought is, to do that in completeness. 
Maybe there is a better wording than refill to indicate the internal copying.


-Ulf

Re: RFR (XS) 8076759: AbstractStringBuilder.append(...) should ensure enough capacity for the upcoming trivial append calls

[Ulf Zibis]

May be:
..., then a new internal array becomes allocated and refilled with greater 
capacity.

... to give a hint, that this action may be expensive.

-Ulf

Am 05.05.2015 um 19:47 schrieb Aleksey Shipilev:

Hi again,

Here is a new webrev:
   http://cr.openjdk.java.net/~shade/8076759/webrev.01/

Pruned the implementation details from expandCapacity Javadoc, and about
to submit a CCC for it.

Thanks,
-Aleksey.

On 05.05.2015 16:33, Roger Riggs wrote:

Hi Aleksey,

Thanks for checking the rounding alternative.

As for the CCC, since the implementation details are in the javadoc
then it will be needed either to remove the details or to update them.
I'd be inclined to try to remove them since they are there primarily for
performance.

Thanks, Roger



On 5/5/2015 4:31 AM, Aleksey Shipilev wrote:

Hi Roger,

On 05/01/2015 08:19 PM, Roger Riggs wrote:

Is there any additional benefit by rounding up the next multiple of 4
or 8.
That would avoid a few wasted bytes at the end of the buffer modulo the
allocation size.

It does not seem to help any further. Tried plus32-round8, as in:
http://cr.openjdk.java.net/~shade/8076759/patches.txt

...and it performs similar to plus32:
http://cr.openjdk.java.net/~shade/8076759/data-foot.png
http://cr.openjdk.java.net/~shade/8076759/data-perf.png


Otherwise, looks fine to me also.

I actually wonder if my change in ensureCapacity Javadoc requires a CCC?
On that topic, I also tempted to remove the implementation details from
the Javadoc there, since it does not play well with describe what you
will do, not how would you do it.

Thanks,
-Aleksey.

Re: RFR (XS) 8076759: AbstractStringBuilder.append(...) should ensure enough capacity for the upcoming trivial append calls

[Ulf Zibis]

Hi Aleksey,

I like this approach and to me the webrev looks good.

-Ulf


Am 24.04.2015 um 20:05 schrieb Aleksey Shipilev:

Hi,

This seems to be a simple one-liner fix, but the background is more
complicated. See the bug:
   https://bugs.openjdk.java.net/browse/JDK-8076759

   http://cr.openjdk.java.net/~shade/8076759/webrev.00/

Re: Places we should use the new ByteBuffer intrinsics

[Ulf Zibis]

Am 16.04.2015 um 15:25 schrieb Paul Sandoz:

They look like good candidates. I did a quick search in the JDK src code 
(usages of getByte/Short/Int/Long) and could not find any others.


I guess there are plenty of candidates in coders of sun.nio.cs.
Additionally: For some coders it may be reasonable to have get3Bytes() and 
put3Bytes() methods.
At the end it wouldn't make me wonder if we could get rid of the if 
(src.hasArray())-fork.

-Ulf

Re: Places we should use the new ByteBuffer intrinsics

[Ulf Zibis]

Am 16.04.2015 um 15:25 schrieb Paul Sandoz:

They look like good candidates. I did a quick search in the JDK src code 
(usages of getByte/Short/Int/Long) and could not find any others.


I guess there are plenty of candidates in coders of sun.nio.cs.
At the end it wouldn't make me wonder if we could get rid of the if 
(src.hasArray())-fork.

-Ulf

Re: RFR: 8073093: AARCH64: C2 generates poor code for ByteBuffer accesses

[Ulf Zibis]

Am 17.02.2015 um 11:53 schrieb Andrew Haley:

On 02/17/2015 10:49 AM, Florian Weimer wrote:

That is, the byte array is supplied by the caller, and if we wanted to
use a ByteBuffer, we would have to allocate a fresh one on every
iteration.  In this case, neither of the two alternatives you list apply.

I see.  So the question could also be whether escape analysis would
notice that a ByteBuffer does not escape.  I hope to know that soon.


See:
https://bugs.openjdk.java.net/browse/JDK-6908239
http://bugs.java.com/bugdatabase/view_bug.do?bug_id=6914113

-Ulf

Re: RFR: 8073093: AARCH64: C2 generates poor code for ByteBuffer accesses

[Ulf Zibis]

Am 17.02.2015 um 04:35 schrieb John Rose:

On Feb 14, 2015, at 12:01 AM, Andrew Haley a...@redhat.com wrote:

On 02/14/2015 12:09 AM, John Rose wrote:

We also need Unsafe.getIntMisaligned, etc., which wire through to whatever 
second-best mechanism the platform offers.

Indeed.  I'm intending to prototype a design for those next week.  OK?

Yes, please.  — John


+1
I guess, also sun.nio.cs coders could benefit from that.

-Ulf

Re: RFR: JDK-8073152: Update Standard/ExtendedCharsets to work with module system

[Ulf Zibis]

Am 16.02.2015 um 19:46 schrieb Xueming Shen:

On 2/16/15 4:11 AM, Alan Bateman wrote:
2. Hasher.java is showing up in the webrev as a new file, was this build.tools.hasher.Hasher and 
so we know have two copies?
This Hasher.java has a different use interface and works on a pair of key/value lists directly 
(instead
of parsing the key/value from a std in). Given the implementation is simple/small enough, I just 
copied/
pasted the parts I need from Mark's code, left the original one untouched (I kinda remember it is 
used

by other as well)


Maybe consider:
https://bugs.openjdk.java.net/browse/JDK-6850361
https://bugs.openjdk.java.net/browse/JDK-6862158
Here have been the patches:
https://bugs.openjdk.java.net/show_bug.cgi?id=100095
https://bugs.openjdk.java.net/show_bug.cgi?id=100098

Thanks,
-Ulf

Re: 8072909: TimSort fails with ArrayIndexOutOfBoundsException on arrays longer than 1073741824

[Ulf Zibis]

Am 11.02.2015 um 23:57 schrieb David Holmes:


So where did the new magic number 49 come from? And how do we know this is now big 
enough?

Thanks,
David


+1
Ulf

Re: Charset.lookupViaProviders uses new ServiceLoader instance on each miss.

[Ulf Zibis]

Am 12.01.2015 um 20:42 schrieb Martin Buchholz:

Historical notes:

I added the two-element cache many years ago, assuming that code that
repeatedly accessed more than 2 charsets would be rare.


I suspect this opinion, see: 
http://bugs.java.com/bugdatabase/view_bug.do?bug_id=6795535

-Ulf

Re: RFR 8060068 : Remove the static initializer block from DriverManager

[Ulf Zibis]

Hi Lance,

to me it's irritating, why there are 2 methods:
- initDriversIfNeeded()
- loadInitialDrivers()
I would combine both to one method.
In lines 90 + 92 there are double spaces.

-Ulf


Am 01.12.2014 um 17:52 schrieb Lance Andersen:

Hi all,

Looking for a review for this change to DriverManager to reduce the possibility on a 
deadlock on clinit. that I have been discussing with Mandy.


The change removes the static initializer block as well as the synchronized 
keyword from registerDriver.

The webrev can be found at 
http://cr.openjdk.java.net/%7Elancea/8060068/webrev.02/


Best,
Lance


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: Review request: JDK-8055723 Replace concat String to append in StringBuilder parameters

[Ulf Zibis]

Hi Otávio,
I now think you could replace
 if (!expected.isEmpty())
with
 assert !expected.isEmpty();

If expected ever would be empty, the only thing which happens is, that a ' is missing in a message 
which anyway doesn't make sense without arguments.


-Ulf



Am 12.11.2014 um 08:19 schrieb Otávio Gonçalves de Santana:

Thank you Ulf.
http://cr.openjdk.java.net/~weijun/8055723/webrev.01/ 
http://cr.openjdk.java.net/%7Eweijun/8055723/webrev.01/


On Sat, Nov 8, 2014 at 3:46 PM, Ulf Zibis ulf.zi...@cosoco.de 
mailto:ulf.zi...@cosoco.de wrote:

Hi Otávio,
in sun/tools/jstat/SyntaxException.java I see a possible enhencement (maybe 
applies to other
places too):

  65 public SyntaxException(int lineno, SetString expected, Token 
found) {
  66 StringBuilder msg = new StringBuilder(A + B * expected.size());
  67
  68 msg.append(Syntax error at line ).append(lineno).append(: 
Expected one of \');
  69
  71 for (String keyWord : expected) {
  72 msg.append(keyWord).append('|');
  73 }
  74 // if (!expected.isEmpty()) // only needed if expected may be 
empty.
  75 msg.setLength(msg.length() - 1);
  76
  81 message = msg.append(\', Found 
).append(found.toMessage()).toString();
  83 }

* Additionally at many places you could similarly introduce the foreach 
syntax.

-Ulf


Am 02.11.2014 um 15:45 schrieb Otávio Gonçalves de Santana:

Could another reviewer look these codes, please.
http://cr.openjdk.java.net/~weijun/8055723/webrev.00/
http://cr.openjdk.java.net/%7Eweijun/8055723/webrev.00/
http://cr.openjdk.java.net/%7Eweijun/8055723/webrev.00/

On Fri, Oct 24, 2014 at 3:25 AM, Otávio Gonçalves de Santana 
otavioj...@java.net
mailto:otavioj...@java.net mailto:otavioj...@java.net 
mailto:otavioj...@java.net wrote:

Thank you Ulf.
I removed the fix in toString method and in debug classes:
http://cr.openjdk.java.net/~weijun/8055723/webrev.00/
http://cr.openjdk.java.net/%7Eweijun/8055723/webrev.00/
http://cr.openjdk.java.net/%7Eweijun/8055723/webrev.00/

On Mon, Oct 20, 2014 at 10:26 PM, Ulf Zibis ulf.zi...@cosoco.de
mailto:ulf.zi...@cosoco.de mailto:ulf.zi...@cosoco.de 
mailto:ulf.zi...@cosoco.de
wrote:


Am 21.10.2014 um 01:02 schrieb Otávio Gonçalves de Santana:

BUGURL: https://bugs.openjdk.java.net/browse/JDK-8055723


WEBREV: 
http://cr.openjdk.java.net/~weijun/8055723/client/webrev.02/
http://cr.openjdk.java.net/%7Eweijun/8055723/client/webrev.02/

http://cr.openjdk.java.net/%7Eweijun/8055723/client/webrev.02/
WEBREV: 
http://cr.openjdk.java.net/~weijun/8055723/core/webrev.03/
http://cr.openjdk.java.net/%7Eweijun/8055723/core/webrev.03/

http://cr.openjdk.java.net/%7Eweijun/8055723/core/webrev.03/


I did not look through all sources.
In Scanner.java I discovered:
1307 sb.append([delimiters=).append(delimPattern).append(']');
1308  sb.append([position=).append(position).append(']');
...
Maybe better:
1307  sb.append([delimiters=).append(delimPattern);
1308  sb.append(][position=).append(position);
...

-Ulf




-- Otávio Gonçalves de Santana

blog: http://otaviosantana.blogspot.com.br/
twitter: http://twitter.com/otaviojava
site: _http://about.me/otaviojava_
55 (11) 98255-3513 tel:55%20%2811%29%2098255-3513 
tel:55%20%2811%29%2098255-3513




-- 
Otávio Gonçalves de Santana


blog: http://otaviosantana.blogspot.com.br/
twitter: http://twitter.com/otaviojava
site: _http://about.me/otaviojava_
55 (11) 98255-3513 tel:55%20%2811%29%2098255-3513 
tel:55%20%2811%29%2098255-3513





--
Otávio Gonçalves de Santana

blog: http://otaviosantana.blogspot.com.br/
twitter: http://twitter.com/otaviojava
site: _http://about.me/otaviojava_
55 (11) 98255-3513

Re: Review request: JDK-8055723 Replace concat String to append in StringBuilder parameters

[Ulf Zibis]

Hi Otávio,
in sun/tools/jstat/SyntaxException.java I see a possible enhencement (maybe applies to other places 
too):


  65 public SyntaxException(int lineno, SetString expected, Token found) {
  66 StringBuilder msg = new StringBuilder(A + B * expected.size());
  67
  68 msg.append(Syntax error at line ).append(lineno).append(: Expected 
one of \');
  69
  71 for (String keyWord : expected) {
  72 msg.append(keyWord).append('|');
  73 }
  74 // if (!expected.isEmpty()) // only needed if expected may be 
empty.
  75 msg.setLength(msg.length() - 1);
  76
  81 message = msg.append(\', Found 
).append(found.toMessage()).toString();
  83 }

* Additionally at many places you could similarly introduce the foreach 
syntax.

-Ulf


Am 02.11.2014 um 15:45 schrieb Otávio Gonçalves de Santana:

Could another reviewer look these codes, please.
http://cr.openjdk.java.net/~weijun/8055723/webrev.00/ 
http://cr.openjdk.java.net/%7Eweijun/8055723/webrev.00/


On Fri, Oct 24, 2014 at 3:25 AM, Otávio Gonçalves de Santana otavioj...@java.net 
mailto:otavioj...@java.net wrote:


Thank you Ulf.
I removed the fix in toString method and in debug classes:
http://cr.openjdk.java.net/~weijun/8055723/webrev.00/
http://cr.openjdk.java.net/%7Eweijun/8055723/webrev.00/

On Mon, Oct 20, 2014 at 10:26 PM, Ulf Zibis ulf.zi...@cosoco.de 
mailto:ulf.zi...@cosoco.de
wrote:


Am 21.10.2014 um 01:02 schrieb Otávio Gonçalves de Santana:

BUGURL: https://bugs.openjdk.java.net/browse/JDK-8055723


WEBREV: http://cr.openjdk.java.net/~weijun/8055723/client/webrev.02/
http://cr.openjdk.java.net/%7Eweijun/8055723/client/webrev.02/
WEBREV: http://cr.openjdk.java.net/~weijun/8055723/core/webrev.03/
http://cr.openjdk.java.net/%7Eweijun/8055723/core/webrev.03/


I did not look through all sources.
In Scanner.java I discovered:
1307 sb.append([delimiters=).append(delimPattern).append(']');
1308  sb.append([position=).append(position).append(']');
...
Maybe better:
1307  sb.append([delimiters=).append(delimPattern);
1308  sb.append(][position=).append(position);
...

-Ulf




-- 
Otávio Gonçalves de Santana


blog: http://otaviosantana.blogspot.com.br/
twitter: http://twitter.com/otaviojava
site: _http://about.me/otaviojava_
55 (11) 98255-3513 tel:55%20%2811%29%2098255-3513




--
Otávio Gonçalves de Santana

blog: http://otaviosantana.blogspot.com.br/
twitter: http://twitter.com/otaviojava
site: _http://about.me/otaviojava_
55 (11) 98255-3513 tel:55%20%2811%29%2098255-3513

Re: Review request: JDK-8055723 Replace concat String to append in StringBuilder parameters

[Ulf Zibis]

Am 21.10.2014 um 01:02 schrieb Otávio Gonçalves de Santana:

BUGURL: https://bugs.openjdk.java.net/browse/JDK-8055723


WEBREV: http://cr.openjdk.java.net/~weijun/8055723/client/webrev.02/
WEBREV: http://cr.openjdk.java.net/~weijun/8055723/core/webrev.03/


I did not look through all sources.
In Scanner.java I discovered:
1307 sb.append([delimiters=).append(delimPattern).append(']');
1308 sb.append([position=).append(position).append(']');
...
Maybe better:
1307 sb.append([delimiters=).append(delimPattern);
1308 sb.append(][position=).append(position);
...

-Ulf

Re: RFR JDK-6321472: Add CRC-32C API

[Ulf Zibis]

Am 17.10.2014 um 20:58 schrieb Staffan Friberg:

Here is a new webrev with the updates from Alan's and Peter's suggestions.
http://cr.openjdk.java.net/~sfriberg/JDK-6321472/webrev.01


I would not remove:
  86 public void update(byte[] b) {
  87 adler = updateBytes(adler, b, 0, b.length);
  88 }

The interfaces default method invokes update(b, 0, b.length), which unnecessarily wastes performance 
with superfluous bound checks, but

  86a if (b == null) {
  86b throw new NullPointerException();
  86c }
should be added, to behave same as
  71 public void update(byte[] b, int off, int len) {
in case of null array.

My 2cc.

-Ulf

Re: RFR (XS) CR 8058643: (str) Re-examine hashCode implementation

[Ulf Zibis]

Am 24.09.2014 um 22:25 schrieb Peter Levart:

Hi,

String.hashCode() caches the result, so repeatable call to same String instance is faster for 2nd 
and further invocations. But the computation of hash code itself could be accelerated for a factor 
of 2 or more on todays CPUs. How? By parallelizing it. And I don't mean computing it in multiple 
threads.


Here is a surprising result of a simple benchmark which computes hash code of a 128 character 
string in 6 different ways:


BenchmarkMode   SamplesScore  Score errorUnits
j.t.HashBench.hashCode  thrpt 8  8420103.795 162447.069ops/s
j.t.HashBench.hashCode0 thrpt 8  8439058.660 2842.755ops/s
j.t.HashBench.hashCode1 thrpt 8 13809510.573 337888.132ops/s
j.t.HashBench.hashCode2 thrpt 8 15543687.568 716152.160ops/s
j.t.HashBench.hashCode3 thrpt 8 18173224.431 49410.256ops/s
j.t.HashBench.hashCode3xthrpt 8  8543020.232 18158.686ops/s


Source:

http://cr.openjdk.java.net/~plevart/misc/StringHash/HashBench.java


This is really great!

Couldn't this be a tweak via HotSpot, instead uglifying and bloating the Java 
and hence the byte code?

-Ulf

Re: Lower overhead String encoding/decoding

[Ulf Zibis]

Compare with https://bugs.openjdk.java.net/browse/JDK-6695386
Maybe that would help a little.

-Ulf


Am 22.09.2014 um 13:25 schrieb Richard Warburton:

Hi all,

A long-standing issue with Strings in Java is the ease and performance of
creating a String from a ByteBuffer. People who are using nio to bring in
data off the network will be receiving that data in the form of bytebuffers
and converting it to some form of String. For example restful systems
receiving XML or Json messages.

The current workaround is to create a byte[] from the ByteBuffer - a
copying action for any direct bytebuffer - and then pass that to the
String. I'd like to propose that we add an additional constructor to the
String class that takes a ByteBuffer as an argument, and directly create
the char[] value inside the String from the ByteBuffer.

Similarly if you have a String that you want to encode onto the wire then
you need to call String.getBytes(), then write your byte[] into a
ByteBuffer or send it over the network. This ends up allocating a byte[] to
do the copy and also trimming the byte[] back down again, usually
allocating another byte[]. To address this problem I've added a couple of
getBytes() overloads that take byte[] and ByteBuffer arguments and write
directly to those buffers.

I've put together a patch that implements this to demonstrate the overall
direction.

http://cr.openjdk.java.net/~rwarburton/string-patch-webrev-5/

I'm happy to take any feedback on direction or the patch itself or the
overall idea/approach. I think there are a number of similar API situations
in other places as well, for example StringBuffer/StringBuilder instances
which could have similar appends directly from ByteBuffer instances instead
of byte[] instances.

I'll also be at Javaone next week, so if you want to talk about this, just
let me know.

regards,

   Richard Warburton

   http://insightfullogic.com
   @RichardWarburto http://twitter.com/richardwarburto

PS: I appreciate that since I'm adding a method to the public API which
consequently requires standardisation but I think that this could get
incorporated into the Java 9 umbrella JSR.

Re: RFR: 8058550: Clarify that TimerTasks are not reusable

[Ulf Zibis]

Am 18.09.2014 um 20:54 schrieb Alan Bateman:

On 18/09/2014 18:52, Martin Buchholz wrote:

Hi Chris,

I'd like you to do a code review.

http://cr.openjdk.java.net/~martin/webrevs/openjdk9/TimerTask-clarification/
https://bugs.openjdk.java.net/browse/JDK-8058550
This looks okay. If I were wording this then I think I might word in the singular, as in Once a 
timer task has been scheduled ... then subsequent attempts to schedule it 


Sounds reasonable. On the other hand, plural indirectly underlines the usual need of multiple timer 
task instances. I let it up to you. in other respects the wording looks okay to me.


I still like to see the extended explanation for the IllegalStateExeption at Timer.schedule(...), 
even if technically redundant, as one might miss the note at TimerTask.


-Ulf

Re: Missing clarity in TimerTask doc

[Ulf Zibis]

Am 16.09.2014 um 00:47 schrieb Martin Buchholz:


Maybe it could be extentend to:
IllegalStateException - if task was already scheduled, cancelled or already 
done, timer was
cancelled, or timer thread terminated.
... as I assume a TimerTask instance is never reusable.


It's already technically correct - if it was already done, then it was 
already scheduled!


Correct!
Note: https://bugs.openjdk.java.net/browse/JDK-8058550


Is there a reason why a TimerTask is not reset for reuse when cancelled or 
already done?


It's a reasonable question.  Like Futures, it's a lot easier to reason about tasks when they 
atomically transition to a done state at most once.  But e.g. FutureTask has runAndReset.


Interesting.

-Ulf

Re: Missing clarity in TimerTask doc

[Ulf Zibis]

Am 15.09.2014 um 22:39 schrieb Martin Buchholz:

http://docs.oracle.com/javase/8/docs/api/java/util/Timer.html#schedule-java.util.TimerTask-long-

IllegalStateException - if task was already scheduled or cancelled, timer was cancelled, or timer 
thread terminated.


Oops, overseen, thanks!

Maybe it could be extentend to:
IllegalStateException - if task was already scheduled, cancelled or already done, timer was 
cancelled, or timer thread terminated.

... as I assume a TimerTask instance is never reusable.

Is there a reason why a TimerTask is not reset for reuse when cancelled or 
already done?

-Ulf





On Mon, Sep 15, 2014 at 10:47 AM, Ulf Zibis ulf.zi...@cosoco.de 
mailto:ulf.zi...@cosoco.de wrote:

Hi all,

I'm missing clarity, if a j.u.TimerTask object can be scheduled more than 
once.
Especially, can it be scheduled again with Timer.schedule(...) after it has 
been cancelled, or
has a new TimerTask object to be instantiated?
What happens if a TimerTask is scheduled again before the first schedule 
had happened; would
it then happen twice or would the same run be re-scheduled?

Thanks,

-Ulf

Re: RFR: 8058230: Improve java.sql toString formatting

[Ulf Zibis]

Maybe consider https://bugs.openjdk.java.net/browse/JDK-6914113
Especially note the last comment there.

-Ulf

Am 11.09.2014 um 16:44 schrieb Claes Redestad:

Hi,

requesting reviews for this patch which optimizes 
java.sql.Date/Time/Timestamp::toString
by avoiding some unnecessary object allocations. java.sql.Date had similar 
optimizations
applied which this patch improves upon.

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

Testing: jtreg jdk/test/java/sql with and without 8057826 tests

Before/after performance running a minimal JMH micro[1]:

Benchmark Mode  Samples  Score Score error   Units
t.DateBench.dateToString thrpt   20 30225.628 623.887  ops/ms
t.DateBench.dateToString thrpt   20 38350.173 1349.432  ops/ms  # 
1.3x

t.DateBench.timeToString thrpt   20 11793.338 232.121  ops/ms
t.DateBench.timeToString thrpt   20 47048.344 1969.939  ops/ms  # 
4.0x

t.DateBench.timestampToStringthrpt   20 2529.601 45.990  ops/ms
t.DateBench.timestampToStringthrpt   20 14143.612 407.351  ops/ms  # 
5.6x

/Claes

[1]

package test;

import org.openjdk.jmh.annotations.*;

@State(Scope.Thread)
public class DateBench {

public java.sql.Time time = java.sql.Time.valueOf(15:15:25);

@Benchmark
public String timeToString() {
return time.toString();
}

public java.sql.Date date = java.sql.Date.valueOf(2013-01-01);

@Benchmark
public String dateToString() {
return date.toString();
}

public Timestamp timestamp = Timestamp.valueOf(1999-12-13 
15:15:25.645634);

@Benchmark
public String timestampToString() {
return timestamp.toString();
}

}

Re: Optimization 2.0 for composing strings - Was: Replace concat String to append in StringBuilder parameters

[Ulf Zibis]

Am 08.09.2014 um 20:53 schrieb Jonathan Gibbons:

For example, in the first few lines of the patch, I found this:

Do you see any semantics change here?


diff -r dde9f5cfde5f 
src/share/classes/sun/tools/jconsole/inspector/XArrayDataViewer.java
--- a/src/share/classes/sun/tools/jconsole/inspector/XArrayDataViewer.java Tue Aug 05 19:29:00 
2014 -0700
+++ b/src/share/classes/sun/tools/jconsole/inspector/XArrayDataViewer.java Sun Aug 10 18:02:01 
2014 -0300

@@ -79,25 +79,24 @@
 String textColor = String.format(%06x,
  foreground.getRGB()  0xFF);
 StringBuilder sb = new StringBuilder();
-sb.append(htmlbody text=#+textColor+table 
width=\100%\);
+sb.append(htmlbody text=#).append(textColor).append(table 
width=\100%\);
 for (int i = 0; i  arr.length; i++) {
 if (i % 2 == 0) {
-sb.append(tr style=\background-color:  +
-evenRowColorStr + \tdpre +
-(arr[i] == null ?
-arr[i] : htmlize(arr[i].toString())) +
- /pre/td/tr);
+sb.append(tr style=\background-color: )
+ .append(evenRowColorStr).append(\tdpre)
+.append(arr[i] == null ?
+arr[i] : htmlize(arr[i].toString()))
+ .append(/pre/td/tr);


In this special case javac optimizer could first interpret as (as same as we could help it by just 
coding like that):

 StringBuilder sb = new StringBuilder(
 htmlbody text=#+textColor+table width=\100%\);
so no need for an additional implicit StringBuilder. Additionally javac could guess a reasonable 
capacity first.


Analyzing the flow to determine it is safe to mutate sb in the face of possible exceptions coming 
out of methods like htmlize is more than it would be reasonable to do in javac.  For example, what 
if the for loop were in a try block and the try block referenced sb?

Good example where such optimization would fail. But:
- manual optimization would fail either
- try catch block anyway would have it's own performance cost, maybe more than the additional 
StringBuilder.


Maybe it's not such complicated as we guess to find out if the affected StringBuilder becomes 
reliably unreachable in case of exception, and therefore securely could be mutated.


Also, consider the serviceability implications, if a user is stepping through the code with a 
debugger.
Good point. But I think, we are already used with this when stepping through simple String 
concatenation or wondering on a StringBuilder in exception stack trace, even there is no 
StringBuilder in our code.



-Ulf

Re: Impact of code difference in Collection#contains() worth improving?

[Ulf Zibis]

Hi Martin,

why you didn't include Mike's suggestion into the fix?

-Ulf

Am 02.09.2014 um 19:04 schrieb Mike Duigou:

Looks fine (bug updated with noreg-perf). I do think we should consider 
promoting a copy of this method to AbstractList as it generally allows for a 
much better implementation than AbstractCollection's contains().

Mike

On Aug 29 2014, at 14:56 , Martin Buchholz marti...@google.com wrote:


Just think - one whole byte saved for each individual change!
I have a webrev!
http://cr.openjdk.java.net/~martin/webrevs/openjdk9/pico-optimize-contains/
https://bugs.openjdk.java.net/browse/JDK-8056951

Can haz review please?


On Fri, Aug 29, 2014 at 1:05 PM, Ulf Zibis ulf.zi...@cosoco.de wrote:


Am 28.08.2014 um 19:46 schrieb Vitaly Davidovich:



There's no register pressure - the immediate (I.e. -1) is encoded
directly into the instruction, just like 0 would be.  The time when 0 is
particularly useful is when you test for it in the zero bit of the flags
register (e.g. dec followed by jz, such as when counting a loop down to
0).  Otherwise, I don't see any advantage from machine code perspective.



Thanks for explaining this, but a very little nit: the immediate (I.e. -1)
uses additional 32/64 bits in code which must be loaded from memory and
wastes space in CPU cache or am I wrong? This could be saved with = 0.

So if unifying the code I agree to Martin's opinion.

-Ulf

The aforementioned cmov instruction is not without its own downsides, so

it's unclear which is better when branch probability isn't known a priori.

The 1 byte code is unlikely to make any difference, unless jit is turned
off and you're running this through a tight loop in the interpreter (but if
one does that, perf conversation is moot :)).

Sent from my phone

On Aug 28, 2014 1:28 PM, Ulf Zibis ulf.zi...@cosoco.de mailto:
ulf.zi...@cosoco.de wrote:


Am 27.08.2014 um 17:51 schrieb Martin Buchholz:

The ArrayList version saves one byte of bytecode, and is
therefore very
slightly better.  We should bless that version and use it
consistently.


+1
Additional argument:
The LinkedList code requires to load 32/64-Bit -1 into CPU. This may
take some time on some
CPU and at least wastes memory footprint.
Additionally register pressure increases.
Vitaly, please correct me, if I'm wrong, just for learning more.

Another advantage is that there is no problem if some implementation
of indexOf() erroneously
returns another negative value than -1. I remember some compare()
implementations, which
sometimes return different values than only -1, 0, +1.

-Ulf

ArrayList:

 public boolean contains(Object o) {
 return indexOf(o) = 0;
 }

LinkedList:

 public boolean contains(Object o) {
 return indexOf(o) != -1;
 }

Re: Impact of code difference in Collection#contains() worth improving?

[Ulf Zibis]

Much thanks for your comments John.

The in bug 6984886 mentioned isHigh/LowSurrogate methods are frequently used in small loops of 
charset de/encoders, so I expect a measurable performance improvement there.


-Ulf


Am 06.09.2014 um 01:25 schrieb John Rose:

On Aug 30, 2014, at 7:17 AM, Ulf Zibis ulf.zi...@cosoco.de wrote:


Am 30.08.2014 um 01:33 schrieb John Rose:

On Aug 29, 2014, at 1:05 PM, Ulf Zibis ulf.zi...@cosoco.de 
mailto:ulf.zi...@cosoco.de wrote:


Thanks for explaining this, but a very little nit: the immediate (I.e. -1) uses 
additional 32/64 bits in code which must be loaded from memory and wastes space in 
CPU cache or am I wrong? This could be saved with = 0.

I have to say you're more wrong than right about this.  Optimizers routinely change the form of constants.  For example, a constant 0 will often 
show up as something like xor eax,eax, not a 32-bit literal zero that loads from somewhere in memory.  A comparison of the form 
x  -1 will be freely changed to x = 0 and back again; the latter form may (or may not, depending on chip version) 
transform to test eax, with no -1 or 0 in sight.

1. Thanks for the hint about x  -1 === x = 0. But I'm afraid this would apply on 
the x != -1 case we are discussing here.

The equality comparison would be less likely to transform to a non-equality comparison.  But 
it is still possible in principle, if the JIT could prove that values x  -1 
are impossible.


2. Are you really sure this optimization is always implemented, as following 
bug is still open:
JDK-6984886 : Transform comparisons against odd border to even border 
http://bugs.java.com/bugdatabase/view_bug.do?bug_id=6984886

Well, I'm pretty sure that no optimizations is always implemented; there are 
usually corner cases which prevent optimizations.  There is always a possibility of 
deoptimization to the interpreter, for example.  And there are lots of nice-to-have 
optimizations that don't make a difference to real applications.  Much of compiler 
development is driven by observing actual speed bottlenecks and removing them.

But the bug reference is useful; thanks!  I added a comment to show where the optimization occurs 
now—disappointingly little I grant you—and where to start looking to improve it.  The bug is rated P5 (lowest 
rating) probably because the reporter said it should be faster; wouldn't this be nice 
is a far weaker argument than I'm spending too much on hardware because this loop is slow.

My comments about the unpredictability of optimizers still stand.  The most 
robust way to manage such problems is, first confirm it is a real performance 
problem (not a micro-nano thing), and second get the optimizer to make all 
equivalent inputs produce good code.  The tiny place where this optimization is 
done in HotSpot suggests that that was a place where the transform actually 
mattered the most.

— John

Re: Optimization 2.0 for composing strings - Was: Replace concat String to append in StringBuilder parameters

[Ulf Zibis]

Much thanks for all your thoughts.

-Ulf

Am 08.09.2014 um 20:53 schrieb Jonathan Gibbons:

Sergey,

Many of the suggestions in the webrev change the semantics of the code, and so would not be 
appropriate for javac to perform automagically.


For example, in the first few lines of the patch, I found this:

diff -r dde9f5cfde5f 
src/share/classes/sun/tools/jconsole/inspector/XArrayDataViewer.java
--- a/src/share/classes/sun/tools/jconsole/inspector/XArrayDataViewer.java Tue Aug 05 19:29:00 
2014 -0700
+++ b/src/share/classes/sun/tools/jconsole/inspector/XArrayDataViewer.java Sun Aug 10 18:02:01 
2014 -0300

@@ -79,25 +79,24 @@
 String textColor = String.format(%06x,
  foreground.getRGB()  0xFF);
 StringBuilder sb = new StringBuilder();
-sb.append(htmlbody text=#+textColor+table 
width=\100%\);
+sb.append(htmlbody text=#).append(textColor).append(table 
width=\100%\);
 for (int i = 0; i  arr.length; i++) {
 if (i % 2 == 0) {
-sb.append(tr style=\background-color:  +
-evenRowColorStr + \tdpre +
-(arr[i] == null ?
-arr[i] : htmlize(arr[i].toString())) +
- /pre/td/tr);
+sb.append(tr style=\background-color: )
+ .append(evenRowColorStr).append(\tdpre)
+.append(arr[i] == null ?
+arr[i] : htmlize(arr[i].toString()))
+ .append(/pre/td/tr);


Analyzing the flow to determine it is safe to mutate sb in the face of possible exceptions coming 
out of methods like htmlize is more than it would be reasonable to do in javac.  For example, what 
if the for loop were in a try block and the try block referenced sb?


Also, consider the serviceability implications, if a user is stepping through the code with a 
debugger.


-- Jon


On 09/08/2014 11:39 AM, Sergey Bylokhov wrote:

On 08.09.2014 21:50, Jonathan Gibbons wrote:
Yes, but is this really a big enough performance and footprint pain point to be worth addressing 
in javac itself?


We're now talking about some specific code construction like
new StringBuilder().append(a + b + c)

Any other case where the string builder can be observed externally cannot be subject to the 
proposed optimization. The use case is now really getting pretty specific.

Not so specific at least in jdk code:
http://mail.openjdk.java.net/pipermail/core-libs-dev/2014-August/028142.html


-- Jon

On 09/08/2014 10:41 AM, Guy Steele wrote:
Good point, but counterpoint: it might be acceptable to have modified the string buffer in 
situations where throwing an exception would always cause the string buffer to become 
inaccessible.


—Guy

On Sep 8, 2014, at 1:30 PM, Jonathan Gibbons jonathan.gibb...@oracle.com 
wrote:


It would be inappropriate/incorrect to apply the optimization as described.

The JLS requires that the argument to a method call should be computed before invoking the 
method.


Consider the case when one of the expressions in the series of string concatenations throws an 
exception. It would be incorrect to have already partially modified the string buffer.


-- Jon

On 08/29/2014 01:53 PM, Ulf Zibis wrote:

Hi compiler people,

is there some chance that javac could be enhanced to optimize better as discussed in this 
thread? Than refactoring of up to now better readable code to ugly StringBuilder@append() 
code would be superfluous.
I really like the String concatenation syntax, but unfortunately it often causes slower code 
and bigger footprint.
Optimally javac would optimize mixed use of StringBuilder, StringJoiner, concatenation, 
toString(), append(String), append(Collection) etc. to a single StringBuilder instance, so 
that the resulting code, JITed or not, will have better performance.

Additionally javac could guess a reasonable initial capacity from the given 
source code.


Am 29.08.2014 um 10:01 schrieb Wang Weijun:

So it's not that the optimization fails but there is no optimization on them 
yet.

I do see the .append(x) case will be easy to deal with, but it looks like historically 
javac has not been a place to do many optimizations. It mostly converts the java source to 
byte codes in a 1-to-1 mapping and let VM do whatever it wants (to optimize). When you 
talked about compiling multiple concatenation into using a single StringBuilder, it's more 
like choosing the correct implementation rather than an optimization.


I don't expect to see big change on this in the near future, so shall we go on with the 
current enhancement?


Thanks
Max

On Aug 29, 2014, at 2:17, Ulf Zibis ulf.zi...@cosoco.de wrote:


I mean:
It does not output byte code that only uses a single char array to compose the entire 
String in question.
With optimization fails, I also mean, there is used an additional StringComposer e.g. 
another StringBuilder or a StringJoiner in addition

Re: Impact of code difference in Collection#contains() worth improving?

[Ulf Zibis]

Am 30.08.2014 um 01:33 schrieb John Rose:

On Aug 29, 2014, at 1:05 PM, Ulf Zibis ulf.zi...@cosoco.de 
mailto:ulf.zi...@cosoco.de wrote:

Thanks for explaining this, but a very little nit: the immediate (I.e. -1) uses additional 32/64 
bits in code which must be loaded from memory and wastes space in CPU cache or am I wrong? This 
could be saved with = 0.


I have to say you're more wrong than right about this.  Optimizers routinely change the form of 
constants.  For example, a constant 0 will often show up as something like xor eax,eax, not a 
32-bit literal zero that loads from somewhere in memory.  A comparison of the form x  -1 will 
be freely changed to x = 0 and back again; the latter form may (or may not, depending on chip 
version) transform to test eax, with no -1 or 0 in sight.


1. Thanks for the hint about x  -1 === x = 0. But I'm afraid this would apply on the x != 
-1 case we are discussing here.

2. Are you really sure this optimization is always implemented, as following 
bug is still open:
JDK-6984886 : Transform comparisons against odd border to even border 
http://bugs.java.com/bugdatabase/view_bug.do?bug_id=6984886


-Ulf

Re: Impact of code difference in Collection#contains() worth improving?

[Ulf Zibis]

Am 28.08.2014 um 19:30 schrieb Louis Wasserman:
Comparator is spec'd to be allowed to return any number, positive, negative, or zero, but indexOf 
is specifically spec'd to return -1.


Yes, I know. I wanted to say, that from this knowing some developer might assume the same for 
indexOf when successfully using ArrayList@contains() with other negative value than -1.



If an indexOf method returns a negative value other than -1, that is a bug;


Yes, but how you suggest to deal with such a bug in existing code. If we would unify JDK code to 
LinkedList style, it would break such existing code. That's what I wanted to point on.


Does the original thread opener want to open a bug e.g. Unify code of 
contains()?
If not, the discussion doesn't make sense anymore.

-Ulf





On Thu, Aug 28, 2014 at 10:27 AM, Ulf Zibis ulf.zi...@cosoco.de 
mailto:ulf.zi...@cosoco.de wrote:


Am 27.08.2014 um 17:51 schrieb Martin Buchholz:

The ArrayList version saves one byte of bytecode, and is therefore very
slightly better.  We should bless that version and use it consistently.


+1
Additional argument:
The LinkedList code requires to load 32/64-Bit -1 into CPU. This may take 
some time on some
CPU and at least wastes memory footprint.
Additionally register pressure increases.
Vitaly, please correct me, if I'm wrong, just for learning more.

Another advantage is that there is no problem if some implementation of 
indexOf() erroneously
returns another negative value than -1. I remember some compare() 
implementations, which
sometimes return different values than only -1, 0, +1.

-Ulf


ArrayList:

 public boolean contains(Object o) {
 return indexOf(o) = 0;
 }

LinkedList:

 public boolean contains(Object o) {
 return indexOf(o) != -1;
 }





--
Louis Wasserman

Re: Impact of code difference in Collection#contains() worth improving?

[Ulf Zibis]

Am 27.08.2014 um 17:51 schrieb Martin Buchholz:

The ArrayList version saves one byte of bytecode, and is therefore very
slightly better.  We should bless that version and use it consistently.


+1
Additional argument:
The LinkedList code requires to load 32/64-Bit -1 into CPU. This may take some time on some CPU and 
at least wastes memory footprint.

Additionally register pressure increases.
Vitaly, please correct me, if I'm wrong, just for learning more.

Another advantage is that there is no problem if some implementation of indexOf() erroneously 
returns another negative value than -1. I remember some compare() implementations, which sometimes 
return different values than only -1, 0, +1.


-Ulf


ArrayList:

 public boolean contains(Object o) {
 return indexOf(o) = 0;
 }

LinkedList:

 public boolean contains(Object o) {
 return indexOf(o) != -1;
 }

Re: Replace concat String to append in StringBuilder parameters

[Ulf Zibis]

I mean:
It does not output byte code that only uses a single char array to compose the entire String in 
question.
With optimization fails, I also mean, there is used an additional StringComposer e.g. another 
StringBuilder or a StringJoiner in addition to the 1st StringBuilder.


-Ulf

Am 27.08.2014 um 14:02 schrieb Pavel Rappo:

Could you please explain what you mean by javac optimization fails here?

-Pavel

On 27 Aug 2014, at 10:41, Ulf Zibis ulf.zi...@cosoco.de wrote:


4.) Now we see, that javac optimization fails again if StringBuilder, 
concatenation, toString(), append(String), append(Collection) etc. and 
StringJoiner use is mixed.

Re: Replace concat String to append in StringBuilder parameters

[Ulf Zibis]

Hi all,

a critical question!

1.) Why we have String concatenation in Java? ... I would answer: for 
_readability_ purpose.

2.) In the early javac times, we were asked to refactor to StringBuffer for 
performance critical code.

3.) Since 1.6 ? javac is capable to replace multiple concatenation with one Stringbuilder, so we are 
again asked to refactor the code for better readability.


4.) Now we see, that javac optimization fails again if StringBuilder, concatenation, toString(), 
append(String), append(Collection) etc. and StringJoiner use is mixed.


*
5.) Couldn't we force the better optimization of javac instead again refactoring the code and 
decrease readability again?

*

+ manual .append(x) should be translated to .append('x')
+ Javac could calculate a reasonable buffer size init value.


-Ulf


Am 11.08.2014 um 16:01 schrieb Ulf Zibis:


Am 11.08.2014 um 15:12 schrieb Andrej Golovnin:

In the most classes I mentioned in my previous mail only the
#toString()-methods would be affected by the proposal. And in the most
cases, maybe in all cases, the #toString()-methods in this classes exists
only to provide nice output.


So why not nice input from the java sources ...i.e.: use concatenation only if possible. The 
performance problem occurs, if both strategies are mixed.



Btw. I see here a nice opportunity to create an RFE
for the Javac team. Following code:

Object o1 = ...;
Object o2 = ...;
String s = abc + o1 + c + o2 + \n;

should be translated to:

String s = new
StringBuilder().append(abc).append(o1).append('c').append(o2).append('\n').toString();

instead of:

String s = new
StringBuilder().append(abc).append(o1).append(c).append(o2).append(\n).toString();


+ manual .append(x) should be translated to .append('x')
+ Javac could avoid to instantiate multiple SBs from mixed concatenation/SB 
source code.
+ Javac could calculate a reasonable buffer size init value.

-Ulf

Re: RFR: 8055949: ByteArrayOutputStream capacity should be maximal array size permitted by VM

[Ulf Zibis]

Am 25.08.2014 um 19:37 schrieb Martin Buchholz:

https://bugs.openjdk.java.net/browse/JDK-8055949
http://cr.openjdk.java.net/~martin/webrevs/openjdk9/ByteArrayOutputStream-MAX_ARRAY_SIZE/

The 2x capacity gap was noticed by real users!


Hi Martin,

the MAX_ARRAY_SIZE code now is copied to many places in JDK. Couldn't you better centralize the code 
to one place, e.g. j.u.Arrays or some hidden class of sun.java...?

Isn't there a property to retrieve MAX_ARRAY_SIZE from the running VM?

Imagine, some VM throws OOME above Integer.MAX_VALUE-4 and minCapacity is 
Integer.MAX_VALUE-4.
With this code a OOME will happen:
 124 return (minCapacity  MAX_ARRAY_SIZE) ?
 125 Integer.MAX_VALUE :
 126 MAX_ARRAY_SIZE;
With this code we would avoid the OOME:
 124 return (minCapacity  MAX_ARRAY_SIZE) ?
 125 minCapacity :
 126 MAX_ARRAY_SIZE;

-Ulf

Re: new StringBuilder(char)

[Ulf Zibis]

Good catch ;-)

-Ulf


Am 08.08.2014 um 22:53 schrieb Eddie Aftandilian:

Re: new StringBuilder(char)

[Ulf Zibis]

Additionally nice to have:
(initial capacity with initial char(s))
StringBuilder(int,char)
StringBuilder(int,CharSequence)

-Ulf


Am 08.08.2014 um 22:53 schrieb Eddie Aftandilian:

Hi all,

We recently realized that calling new StringBuilder(char) does not do what
you would think it does.  Since there is no char constructor defined, the
char is widened to an int and the StringBuffer is presized to the
character's encoded value.  Thus code like this prints an empty string
rather than the expected a:
System.out.println(new StringBuilder('a'));

Would it be possible to add a char constructor to StringBuilder to prevent
this problem? I understand this would change the behavior of any code that
is currently doing this, but it's hard to imagine anyone doing this
intentionally.  Of the ~20 instances we found in Google's codebase, all
were bugs.  What is your policy on making changes like this where (a) it
will cause a change in behavior, but (b) the currently behavior is clearly
wrong?

If you're willing to take the change, I'd be happy to send a patch.

Thanks,
Eddie

Re: Replace concat String to append in StringBuilder parameters

[Ulf Zibis]

Am 11.08.2014 um 15:12 schrieb Andrej Golovnin:

In the most classes I mentioned in my previous mail only the
#toString()-methods would be affected by the proposal. And in the most
cases, maybe in all cases, the #toString()-methods in this classes exists
only to provide nice output.


So why not nice input from the java sources ...i.e.: use concatenation only if possible. The 
performance problem occurs, if both strategies are mixed.



Btw. I see here a nice opportunity to create an RFE
for the Javac team. Following code:

Object o1 = ...;
Object o2 = ...;
String s = abc + o1 + c + o2 + \n;

should be translated to:

String s = new
StringBuilder().append(abc).append(o1).append('c').append(o2).append('\n').toString();

instead of:

String s = new
StringBuilder().append(abc).append(o1).append(c).append(o2).append(\n).toString();


+ manual .append(x) should be translated to .append(x)
+ Javac could avoid to instantiate multiple SBs from mixed concatenation/SB 
source code.
+ Javac could calculate a reasonable buffer size init value.

-Ulf

Re: Replace concat String to append in StringBuilder parameters

[Ulf Zibis]

Am 11.08.2014 um 16:33 schrieb Pavel Rappo:

Unfortunately, neither java.util.StringJoiner nor String.join have perfect (but 
who has?) APIs. So it's up to us to figure out the best way of joining elements.


Maybe remember my thoughts from here:
http://mail.openjdk.java.net/pipermail/core-libs-dev/2013-April/016172.html
http://mail.openjdk.java.net/pipermail/core-libs-dev/2014-January/024761.html

Would it be possible to inherit StringJoiner from StringBuilder in some way?
Then a mixture of concatanation, StringBuilder and StringJoiner after Javac would end up in one 
StringBuilder instance. We also could profit from StringJoiners initial capacity capability.


-Ulf

Re: Replace concat String to append in StringBuilder parameters

[Ulf Zibis]

Hi Otávio,

this is a great proposal.

Little nit:
In cases where only 1 character is to append, I guess append(char) would be faster and at least will 
save footprint in contrast to append(String).


-Ulf


Am 10.08.2014 um 23:33 schrieb Otávio Gonçalves de Santana:

*Motivation:* Make another append instead of concat String inside of append
parameter in StringBuilder class. To avoid an extra StringBuilder created
for the purpose of concatenating. So it will save memory and will faster
than concat String.
Doing a code to benchMark[1], the result is:

Benchmark  Mode   Samples
   Mean   Mean errorUnits
m.StringBuilderConcatBenchMark.stringBuilder  thrpt10
  6317444.705   108673.584ops/s
m.StringBuilderConcatBenchMark.stringBuilderWithConcatthrpt10
  3354554.43568353.924ops/s

The webrev of all code is:
https://dl.dropboxusercontent.com/u/16109193/open_jdk/string_builder_concat.zip
https://dl.dropboxusercontent.com/u/16109193/open_jdk/string_builder_concat.zip

[1]

@State(Scope.Thread)
@OutputTimeUnit(TimeUnit.SECONDS)
public class StringBuilderConcatBenchMark {


 private static final String F = ;
  private static final String E =  running in Java ;
private static final String D =  in the code ;
  private static final String C =  to try impact ;
private static final String B =  with some text ;
  private static final String A = Doing a test;

@GenerateMicroBenchmark
public void stringBuilder(BlackHole bh) {
  bh.consume(createBuilder(A, B, C, D, E, F));
}

@GenerateMicroBenchmark
  public void stringBuilderWithConcat(BlackHole bh) {
bh.consume(createBuilderWithConcat(A, B, C, D, E, F));
  }

private StringBuilder createBuilder(String... values) {
StringBuilder text = new StringBuilder();
  text.append(values[0]).append(values[1])
.append(values[2]).append(values[3])
.append(values[4]).append(values[5]);
  return text;
}
  private StringBuilder createBuilderWithConcat(String... values) {
  StringBuilder text = new StringBuilder();
text.append(values[0] + values[1])
.append(values[2] + values[3])
  .append(values[4]+ values[5]);
return text;
}
}

Re: Replace concat String to append in StringBuilder parameters

[Ulf Zibis]

Am 11.08.2014 um 01:03 schrieb Claes Redestad:

- in places like src/share/classes/sun/security/provider/PolicyFile.java
  you end up with a sequence of String literal appends which could be merged 
into one:

-sb.append(principalInfo[i][0] +   +
-\ + principalInfo[i][1] + \);
+sb.append(principalInfo[i][0]).append( \)
+.append(principalInfo[i][1]).append('');


In cases of only 2 chars, I can imagine, that .append(' ').append('\') would 
have better performance.

-Ulf

Re: RFR: JDK-8042589: String.toLowerCase do not work for some concatenated strings

[Ulf Zibis]

There is again another little optimization possible:

instead
hasSurr = true;
we could use
first = ~first;
and save variable hasSurr.

If there is register pressure (especially on older CPUs), this might be a 
performance advantage.

Also I think we should not grow the result array for any character where mapLen  srcCount, we 
could wait until the result array is actually full.


-Ulf


Am 17.07.2014 00:55, schrieb Xueming Shen:

Still need a reviewer.

On 07/09/2014 01:04 PM, Xueming Shen wrote:

Hi,

Please help review the change for JDK-8042589.

Issue:https://bugs.openjdk.java.net/browse/JDK-8042589
webrev: http://cr.openjdk.java.net/~sherman/8042589/webrev/

This is a regression caused by the following change for #JDK-8032012,

issue:https://bugs.openjdk.java.net/browse/JDK-8032012
webrev: http://cr.openjdk.java.net/~sherman/8032012/
discussion: 
http://mail.openjdk.java.net/pipermail/core-libs-dev/2014-February/024862.html

It appears the last optimization for the surrogates we pushed in is
incomplete. We still need to check isSurrogate() in the optimized
non-surrogate loop, as the first (checked at the very beginning) might
be triggered by a non-surrogate-upper/lowercase char.

Thanks!
-Sherman

Re: RFR: JDK-8042589: String.toLowerCase do not work for some concatenated strings

[Ulf Zibis]

Additionally I think, instead of retrieving
String lang = locale.getLanguage()
multiple times in chain, we could pass lang to the methods instead locale.

-Ulf

Am 17.07.2014 11:54, schrieb Ulf Zibis:

There is again another little optimization possible:

instead
hasSurr = true;
we could use
first = ~first;
and save variable hasSurr.

If there is register pressure (especially on older CPUs), this might be a 
performance advantage.

Also I think we should not grow the result array for any character where mapLen  srcCount, we 
could wait until the result array is actually full.


-Ulf


Am 17.07.2014 00:55, schrieb Xueming Shen:

Still need a reviewer.

On 07/09/2014 01:04 PM, Xueming Shen wrote:

Hi,

Please help review the change for JDK-8042589.

Issue:https://bugs.openjdk.java.net/browse/JDK-8042589
webrev: http://cr.openjdk.java.net/~sherman/8042589/webrev/

This is a regression caused by the following change for #JDK-8032012,

issue:https://bugs.openjdk.java.net/browse/JDK-8032012
webrev: http://cr.openjdk.java.net/~sherman/8032012/
discussion: 
http://mail.openjdk.java.net/pipermail/core-libs-dev/2014-February/024862.html

It appears the last optimization for the surrogates we pushed in is
incomplete. We still need to check isSurrogate() in the optimized
non-surrogate loop, as the first (checked at the very beginning) might
be triggered by a non-surrogate-upper/lowercase char.

Thanks!
-Sherman

Re: Covariant overrides on the Buffer Hierachy

[Ulf Zibis]

Am 02.05.2014 18:13, schrieb Joe Darcy:

On 5/2/2014 9:10 AM, Ulf Zibis wrote:

Am 01.05.2014 02:20, schrieb Joe Darcy:
If from an API perspective the new code is preferable, I would say that should take precedence 
over an at most marginal performance degradation.


I guess you meant: ... marginal performance _gain_.



No, I actually meant degradation.

If the new API is easier to use, captures more type information, etc., in my estimation that is 
more important than a whisker thin performance penalty in an area that is not performance critical.


Hi Joe,

Ok I see, I have stumbled over the semantical grammar of your sentence. English is not my mother 
tongue. You set the benefit and the disadvantage of 1 approach in concurrence.


I have read the concurrence of 2 possible benefits of the 2 possible approaches:
- better API
- better performance

Thanks, Ulf

Re: Covariant overrides on the Buffer Hierachy

[Ulf Zibis]

Am 01.05.2014 02:20, schrieb Joe Darcy:

Hello,

I'm reminded of Professor Knuth's observation that Premature optimization is the 
root of all evil.

If from an API perspective the new code is preferable, I would say that should take precedence 
over an at most marginal performance degradation.


I guess you meant: ... marginal performance _gain_.

-Ulf

Re: JDK 9 RFR of 8039474: sun.misc.CharacterDecoder.decodeBuffer should use getBytes(iso8859-1)

[Ulf Zibis]

Am 10.04.2014 21:53, schrieb Tim Bell:

On 04/10/14 19:26, Ulf Zibis wrote:

BTW, where are these links gone:


This part of the question I can handle.

The six digit Bug numbers came from the legacy OpenJDK bugzilla instance.

Before it was shut down, those bug reports were transferred to JBS. In the 
process, they were
assigned new JDK-nnn bug numbers, so you will be able to view them on the 
new system:

In general, if you saved an old Bugzilla ID (six digits, for example 100092), 
you should be able
to find it in JBS by visiting this URL:

  https://bugs.openjdk.java.net/issues/?jql=

And doing a simple search for the string id=100092


Hope this helps-


Much thanks Tim.

But where are the original attachments e.g. webrevs, patches ?
Are they lost forever ?

-Ulf

Re: Covariant overrides on the Buffer Hierachy

[Ulf Zibis]

Hi,

Am 15.04.2014 10:05, schrieb Richard Warburton:

Hi,

I'd like to have a discussion about tidying up a few core library method
signatures in a way that (I think) is backwards compatible.

I've been using ByteBuffer quite a lot recently which is designed to be a
fluent API. Unfortunately its quite inconvenient to use because there's a
hierarchy of classes at play (MappedByteBuffer, ByteBuffer and Buffer) and
methods which are inherited from parent classes aren't overridden with
covariant return types. For example the following code doesn't compile
because clear is defined on Buffer and putInt is defined on ByteBuffer.

ByteBuffer buffer = ByteBuffer.allocate(256);

ByteBuffer other = buffer.duplicate()
  .clear()
  .putInt(0, 4);

If clear was overridden in ByteBuffer with ByteBuffer as its return type
then this would compile.


See also:

http://mail.openjdk.java.net/pipermail/core-libs-dev/2009-April/001512.html
http://mail.openjdk.java.net/pipermail/core-libs-dev/2009-April/001365.html
http://mail.openjdk.java.net/pipermail/coin-dev/2009-March/001180.html

https://bugs.openjdk.java.net/browse/JDK-6472931
https://bugs.openjdk.java.net/browse/JDK-6373386
https://bugs.openjdk.java.net/browse/JDK-6479372
https://bugs.openjdk.java.net/browse/JDK-4774077
https://bugs.openjdk.java.net/browse/JDK-6451131
https://bugs.openjdk.java.net/browse/JDK-5082736

-Ulf

Re: JDK 9 RFR of 8039474: sun.misc.CharacterDecoder.decodeBuffer should use getBytes(iso8859-1)

[Ulf Zibis]

Hi Chris,

Am 10.04.2014 11:04, schrieb Chris Hegarty:

Trivially, you could ( but of not have to ) use 
java.nio.charset.StandardCharsets.ISO_8859_1 to avoid the cost of String to 
CharSet lookup.


In earlier tests Sherman and I have found out, that the cost of initialization of a new charsets 
object is higher than the lookup of an existing object in the cache.

And it's even better to use the same String instance for the lookup which was 
used to cache the charset.

-Ulf

Re: JDK 9 RFR of 8039474: sun.misc.CharacterDecoder.decodeBuffer should use getBytes(iso8859-1)

[Ulf Zibis]

Correction ...

Am 10.04.2014 12:03, schrieb Ulf Zibis:

Hi Chris,

Am 10.04.2014 11:04, schrieb Chris Hegarty:
Trivially, you could ( but of not have to ) use java.nio.charset.StandardCharsets.ISO_8859_1 to 
avoid the cost of String to CharSet lookup.


In earlier tests Sherman and I have found out, that the cost of initialization of a new charsets 
object is higher than the lookup of an existing object in the cache.
And it's even better to use the same String instance for the lookup which was used to cache the 
charset.


It's not about the cached charset, but about the cached charsets de/encoder, 
compare:
StringCoding.decode(String charsetName, byte[] ba, int off, int len)
StringCoding.decode(Charset cs, byte[] ba, int off, int len)

-Ulf

Re: JDK 9 RFR of 8039474: sun.misc.CharacterDecoder.decodeBuffer should use getBytes(iso8859-1)

[Ulf Zibis]

Am 10.04.2014 17:20, schrieb Xueming Shen:

Looks fine. Personally I would prefer the canonicalized/real  name 
ISO-8859-1 though.


Yep, using the canonical name guarantees best performance for the charset 
lookup.

BTW, where are these links gone:
Bug 100092 -- Speed-up FastCharsetProvider 
https://bugs.openjdk.java.net/show_bug.cgi?id=100092
Bug 100095 -- Avoid 2-step lookup in sun.nio.cs charset providers 
https://bugs.openjdk.java.net/show_bug.cgi?id=100095
Bug 100098 -- Make sun.nio.cs.* charset objects light-weight 
https://bugs.openjdk.java.net/show_bug.cgi?id=100098


-Ulf

Re: JDK 9 RFR of 8039474: sun.misc.CharacterDecoder.decodeBuffer should use getBytes(iso8859-1)

[Ulf Zibis]

Am 10.04.2014 21:53, schrieb Tim Bell:

On 04/10/14 19:26, Ulf Zibis wrote:

BTW, where are these links gone:


This part of the question I can handle.

The six digit Bug numbers came from the legacy OpenJDK bugzilla instance.

Before it was shut down, those bug reports were transferred to JBS. In the process, they were 
assigned new JDK-nnn bug numbers, so you will be able to view them on the new system:


In general, if you saved an old Bugzilla ID (six digits, for example 100092), you should be able 
to find it in JBS by visiting this URL:


  https://bugs.openjdk.java.net/issues/?jql=

And doing a simple search for the string id=100092


Hope this helps-


Much thanks Tim.

But where are the original attachments e.g. webrevs, patches ?

-Ulf

Re: Implicit 'this' return for void methods

[Ulf Zibis]

Hi,

Am 02.04.2014 11:08, schrieb Andrew Haley:

On 04/01/2014 10:20 PM, Eirik Lygre wrote:

What is the relationship between this naked dot proposal and the
chaining of void methods proposal? The reason for asking is not to be
negative, but rather to find out if these issues are best dealt with
together, or as independent proposals.

I think that if either of these are going to happen, then they must be
specified with the appropriate level of isolation: That which belongs
together must be processed together; that which belongs apart must be
processed apart.

Point taken, but Project Coin (small language changes) worked well.

Andrew.


If that would help to make things happen, I support the idea to separate both steps to different 
proposals.


-Ulf

Re: Implicit 'this' return for void methods

[Ulf Zibis]

Am 01.04.2014 11:28, schrieb Bruce Chapman:

Slightly preceding Ulf's coin proposal by a few hours was

http://mail.openjdk.java.net/pipermail/coin-dev/2009-March/001134.html

Where I suggested the naked dot notation (coined in 
http://mail.openjdk.java.net/pipermail/coin-dev/2009-March/000855.html) has better value as .. a

syntax for referring to the receiver of a method inside arguments to the
method.

More formally, the naked dot  (at the start of an expression, not following an invocation to a 
void method) would refer to the receiver of the innermost surrounding invocation expression.


and so to answer Guy's question below in terms of my original intention rather than Ulf's 
proposal, .indexof(Q) would use myVeryLongNamedString as its receiver.


+1
My proposal was meant exactly as that. Maybe my wording was not clear enough in 
that.

-Ulf

Re: Implicit 'this' return for void methods

[Ulf Zibis]

Hi Guy and Paul,

thanks for liking my proposal.

What can we do to convince the officials ?

-Ulf


Am 26.03.2014 17:20, schrieb Paul Benedict:
It would be nice to make this language change. In the past years, it's pretty clear many JSR EE 
spec leads have gone on to make their APIs return the same object because they strongly prefer to 
see object chaining in code. I sympathize with those designers, but I don't agree; I wouldn't 
affect my API just for the sake of chaining. For the sake of clarity, I prefer functions that 
don't compute anything to return void. So +1 for the language to figure this out.



On Wed, Mar 26, 2014 at 10:51 AM, Guy Steele guy.ste...@oracle.com 
mailto:guy.ste...@oracle.com wrote:



On Mar 26, 2014, at 4:17 AM, Ulf Zibis ulf.zi...@cosoco.de wrote:

 See also:
 . . .
 http://mail.openjdk.java.net/pipermail/coin-dev/2009-March/001180.html

This last one has a specific proposal, which is simple and quite nice.  The 
important idea is
that we don't actually make any change to the code of void methods or make 
them actually
return anything; instead, the caller takes notice of situations where an 
invocation of
a void method is used as a subexpression whose value is required 
(heretofore forbidden
by the language) and gives it a special interpretation.

I note that Ulf's proposal applies only to method invocations, but I note 
that the same
technique could be used to include field references if desired.

I am wholeheartedly in favor of allowing chaining of dotted expressions 
such as

 CharBuffer.allocate(26).position(2).put(C).position(25).put(Z)

I am a bit more skeptical about expressions that begin with a dot because 
of potential
confusion about which expression is referred to:

myVeryLongNamedString.subString(.indexOf(C), .indexOf(Q))

seems clear enough, but what about:

myVeryLongNamedString.subString(.indexOf(C) + otherString.length(), 
.indexOf(Q))

Does the second occurrence of .indexOf use myVeryLongNamedString or 
otherString?

A compromise would be to allow leading-dot expressions to occur only within 
the arguments
of the method call whose target is the object which the leading-dot 
expressions are expected
to use as their target, and if there are such leading-dot expressions 
within the arguments
then the arguments must not contain any non-leading-dot field references or 
method calls.
Just a thought for discussion.  This would be considered a separate 
mechanism from the
chaining-of-void-methods mechanism (it was a very clever idea to try to 
unify them in Ulf's
original proposal, though).

---Guy




--
Cheers,
Paul

Re: Implicit 'this' return for void methods

[Ulf Zibis]

Am 27.03.2014 23:05, schrieb Eirik Lygre:
With this suggested change, the only behavior that will change is that some code which used to not 
compile will start compiling, with a reasonable result. No code that used to compile will change.


Yes, this is one of the great advantages of this simple _language_ change 
proposal !

-Ulf

Re: Implicit 'this' return for void methods

[Ulf Zibis]

See:
https://bugs.openjdk.java.net/browse/JDK-6472931
https://bugs.openjdk.java.net/browse/JDK-6373386
https://bugs.openjdk.java.net/browse/JDK-6479372
https://bugs.openjdk.java.net/browse/JDK-4774077
https://bugs.openjdk.java.net/browse/JDK-6451131
https://bugs.openjdk.java.net/browse/JDK-5082736

-Ulf


Am 25.03.2014 21:19, schrieb Victor Polischuk:

Good day, everyone,

I have a question regarding a proposal I found some time ago: 
http://tech.puredanger.com/java7#chained.

In a word, it is all about replacing 'void' behavior to always return 'this' 
and I think for static methods it would be nice to return Class instance. I 
have not found any mentions in JEPs and just wonder if the idea is rejected or 
still somewhere in pipeline for Java 1.x. (or perhaps it is just waiting to be 
filed by someone in brand new JEP).

---
Regards,
Victor Polischuk

Re: Implicit 'this' return for void methods

[Ulf Zibis]

See also:

http://mail.openjdk.java.net/pipermail/core-libs-dev/2009-April/001512.html
http://mail.openjdk.java.net/pipermail/core-libs-dev/2009-April/001365.html
http://mail.openjdk.java.net/pipermail/coin-dev/2009-March/001180.html

-Ulf

Am 25.03.2014 21:19, schrieb Victor Polischuk:

Good day, everyone,

I have a question regarding a proposal I found some time ago: 
http://tech.puredanger.com/java7#chained.

In a word, it is all about replacing 'void' behavior to always return 'this' 
and I think for static methods it would be nice to return Class instance. I 
have not found any mentions in JEPs and just wonder if the idea is rejected or 
still somewhere in pipeline for Java 1.x. (or perhaps it is just waiting to be 
filed by someone in brand new JEP).

---
Regards,
Victor Polischuk

Re: RFR [8014066] Mistake in documentation of ArrayList#removeRange

[Ulf Zibis]

Am 19.03.2014 23:37, schrieb Ulf Zibis:

Am 19.03.2014 23:32, schrieb Martin Buchholz:

No one is as performance obsessed as Ulf.


:-D :-D :-D


And additionally footprint obsessed

-Ulf

Re: RFR [8014066] Mistake in documentation of ArrayList#removeRange

[Ulf Zibis]

Am 19.03.2014 09:19, schrieb Ivan Gerasimov:

Thank you Ulf!

On 19.03.2014 2:12, Ulf Zibis wrote:

Am 18.03.2014 19:28, schrieb Ivan Gerasimov:


Assuming this last iteration is OK, should the next step be a CCC request?


Do you mean? :
 /*
  * ...
+ * It is assumed that fromIndex = toIndex, otherwise the behaviour of this method is 
undefined.

  * ...
 - *  toIndex  fromIndex})
  * ...
 */
 protected void removeRange(int fromIndex, int toIndex) {
 ...



The (fromIndex  toIndex) condition is checked now, so the behavior of the method is defined - it 
throws an exception.


Because Martin stated some days ago, that it normally should not occur, that removeRange is invoked 
with toIndex  fromIndex, and checking this again unnecessarily decreases performance a little, I 
was hoping, your next CCC step would be to drop this check.


-Ulf

Re: RFR [8014066] Mistake in documentation of ArrayList#removeRange

[Ulf Zibis]

Am 19.03.2014 23:32, schrieb Martin Buchholz:

No one is as performance obsessed as Ulf.


:-D :-D :-D

Re: RFR [8014066] Mistake in documentation of ArrayList#removeRange

[Ulf Zibis]

Am 18.03.2014 19:28, schrieb Ivan Gerasimov:


Assuming this last iteration is OK, should the next step be a CCC request?


Do you mean? :
 /*
  * ...
+ * It is assumed that fromIndex = toIndex, otherwise the behaviour of 
this method is undefined.
  * ...
 - *  toIndex  fromIndex})
  * ...
 */
 protected void removeRange(int fromIndex, int toIndex) {
 ...

Please remove and replace inline by size - toIndex:
 621 int numMoved = size - toIndex;


About modCount:

Wouldn't it be more correct to code? :

 616 if (fromIndex  toIndex) {
 617 throw new IndexOutOfBoundsException(
 618 outOfBoundsMsg(fromIndex, toIndex));
 619 }
 620 try {
 621 modCount++;
 622 System.arraycopy(elementData, toIndex, elementData, fromIndex,
 623 size - toIndex);
 624 } catch (IndexOutOfBoundsException ioobe) {
 625 modCount--;
 626 throw ioobe;
 627 }

Of even better :

 000 private int[] modCount = { 0 };
 ...
 616 if (fromIndex  toIndex) {
 617 throw new IndexOutOfBoundsException(
 618 outOfBoundsMsg(fromIndex, toIndex));
 619 }
 620 System.arraycopy(elementData, toIndex, elementData, fromIndex,
 621 size - toIndex, modCount);

Or :

 000 public class ArrayList ... implements ModCounter {
 001 private int modCount = 0;
 001 void incModCount() {
 001 modCount++;
 004}
 ...
 616 if (fromIndex  toIndex) {
 617 throw new IndexOutOfBoundsException(
 618 outOfBoundsMsg(fromIndex, toIndex));
 619 }
 620 System.arraycopy(elementData, toIndex, elementData, fromIndex,
 621 size - toIndex, this);

-Ulf

Re: RFR [8014066] Mistake in documentation of ArrayList#removeRange

[Ulf Zibis]

Am 18.03.2014 23:17, schrieb Martin Buchholz:
modCount is an imprecise concurrent modification mechanism.  It doesn't have to be kept 
transactionally correct.


Thanks, yes I know.
But does it hurt to make it more precise?
See this as a concept for a RFE.

-Ulf

Re: RFR 8037106: Optimize Arrays.asList(...).forEach

[Ulf Zibis]

Am 17.03.2014 17:36, schrieb Paul Sandoz:

Hi Sergey,

Thanks, you are right! I did not realize it copied the array into a local 
variable, but that makes sense.

Here is the byte code generated by javac (9) for two different methods:
.


Thanks from me too, this is great work.

I floated my question because of ...

Am 14.03.2014 16:46, schrieb Martin Buchholz:

+final int size = a.length;
+for (int i = 0; i  size; i++) {

I like to write this using the idiom

for (int i = 0, size = ...; i  size; i++)


This is not thread-proof (Given there is no: final E[] a = this.a;). If a becomes replaced 
concurrently while looping, size becomes invalid as length of a.


Good to know, that foreach over array is always thread-proof and additionally performs at least 
equal (except of the initial local caching) than a plain fortran-style loop.


-Ulf

Re: RFR 8037106: Optimize Arrays.asList(...).forEach

[Ulf Zibis]

Am 17.03.2014 17:08, schrieb mark.reinh...@oracle.com:

2014/3/17 1:41 -0700, paul.san...@oracle.com:

On Mar 15, 2014, at 12:17 AM, Ulf Zibis ulf.zi...@cosoco.de wrote:

...

I more like the given style with less spaces:
3854 for (int i=0; ia.length; i++)
It better visualizes the 3 parts of the for statement.


Subjectively that irritates my eyes :-) non-subjectively it is
inconsistently applied.


What you mean by inconsistently? In the JDK sources?


It's also, well, just plain wrong.

- Mark


You are correct from java code conventions.

But I'm wondering, why this wrong style is such popular, even in JDK, at least in older sources - 
where IDE-driven auto-formatting was not available ;-)
My only explanation: People think, it's better readable, it better visualizes the 3 parts of the for 
statement.


BTW: IIRC, in NetBeans both styles are configurable for auto-formatting.

-Ulf

Another similar case: https://bugs.openjdk.java.net/browse/JDK-6939278

Re: RFR [8014066] Mistake in documentation of ArrayList#removeRange

[Ulf Zibis]

Am 16.03.2014 23:37, schrieb Ivan Gerasimov:

Here is yet another iteration of the fix:
http://cr.openjdk.java.net/~igerasim/8014066/3/webrev/

2)
Kept the check for 'fromIndex  toIndex' in removeRange().
While I understand that this should not add anything significant to the current code, as currently 
removeRange() is always called with valid arguments.
However, if it is stated in the spec that in case of 'fromIndex  toIndex' an exception is thrown, 
I believe it should be thrown, otherwise why it's stated?


Isn't this a perfect situation to use an assert statement? Needless to say, then the spec must be 
adapted.


-Ulf

Re: RFR 8037106: Optimize Arrays.asList(...).forEach

[Ulf Zibis]

Am 14.03.2014 17:10, schrieb Paul Sandoz:

I'm willing to believe for-loop over array is as efficient as fortran-style loop

+for (E e : a) {
+action.accept(e);
+}


Yeah, i previously went through a whole bunch of code replacing such 
fortran-style loops with 'foreach' style based on automated code analysis.


But wouldn't this help a little more? :
+final E[] a = this.a;
+for (E e : a) {
+action.accept(e);
+}


I more like the given style with less spaces:
3854 for (int i=0; ia.length; i++)
It better visualizes the 3 parts of the for statement.

-Ulf

Re: Unsafe: efficiently comparing two byte arrays

[Ulf Zibis]

Am 28.02.2014 23:29, schrieb Martin Buchholz:

Are word-size reads in ByteBuffer actually intrinsified?  I could find no
evidence for that.  Perhaps this is an important missing optimization for
hotspot to make?

I see DirectByteBuffer, but not garden-variety ByteBuffer, using Unsafe.
share/classes/java/nio/Direct-X-Buffer.java.template

Probably should be investigated using disassembly of jitted code, which I
have never done.


See also:
https://bugs.openjdk.java.net/browse/JDK-6914113

-Ulf

Re: RFR: JDK-8032012, , String.toLowerCase/toUpperCase performance improvement

[Ulf Zibis]

Am 06.02.2014 18:40, schrieb Xueming Shen:

Are we good enough to go? :-) While it took much longer than I would have
expected, but I'm happy with the latest result.

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


Except if (first = len) instead if (first = len) I'm with you.

-Ulf