Re: RFR: 8248655: Support supplementary characters in String case insensitive operations

[James Laskey]





> On Jul 15, 2020, at 4:32 PM, Joe Wang  wrote:
> 
> Jim: I was referring to the rest of the process (the calls to 
> toCodePoint/toUpperCase/toLowerCase), not isHighSurrogate itself. But Roger 
> has a more comprehensive review on performance, and Naoto is planning on 
> preparing a JMH test.
> 
> -Joe
> 
>> On 7/15/2020 11:46 AM, Jim Laskey wrote:
>> Joe: This is a defensive approach that I believe has minimal cost.
>> 
>> public static boolean isHighSurrogate(char ch) {
>> // Help VM constant-fold; MAX_HIGH_SURROGATE + 1 == MIN_LOW_SURROGATE
>> return ch >= MIN_HIGH_SURROGATE && ch < (MAX_HIGH_SURROGATE + 1);
>> }
>> 
>> 
 On Jul 15, 2020, at 3:32 PM, naoto.s...@oracle.com wrote:
>>> 
>>> Hi Joe,
>>> 
>>> Thank you for your review.
>>> 
>>> On 7/15/20 10:57 AM, Joe Wang wrote:
 Hi Naoto,
 In StringUTF16.java, if one is isHighSurrogate and the other not, you may 
 quickly return without going through the rest of the process, probably not 
 significant as cp1 and cp2 and/or u1 and u2 won't be equal anyways. But it 
 could skip a couple of toCodePoint/toUpperCase/toLowerCase calls.
>>> Yes, that is correct as of now, which is based on the assumption that case 
>>> mappings do not cross BMP and supplementary planes boundary. I could not 
>>> find any description where that's given or not. So I just took it to be 
>>> safe.
>>> 
>>> Naoto
>>> 
 -Joe
 On 7/15/20 9:00 AM, naoto.s...@oracle.com wrote:
> Hello,
> 
> Please review the fix to the following issues:
> 
> https://bugs.openjdk.java.net/browse/JDK-8248655
> https://bugs.openjdk.java.net/browse/JDK-8248434
> 
> The proposed changeset and its CSR are located at:
> 
> https://cr.openjdk.java.net/~naoto/8248655.8248434/webrev.00/
> https://bugs.openjdk.java.net/browse/JDK-8248664
> 
> A bug was filed against SimpleDateFormat (8248434) where case-insensitive 
> date format/parse failed in some of the new locales in JDK15. The root 
> cause was that case-insensitive String.regionMatches() method did not 
> work with supplementary characters. The problem is that the method's spec 
> does not expect case mappings of supplementary characters, possibly 
> because it was overlooked in the first place, JSR 204 - "Unicode 
> Supplementary Character support". Similar behavior is observed in other 
> two case-insensitive methods, i.e., compareToIgnoreCase() and 
> equalsIgnoreCase().
> 
> The fix is straightforward to compare strings by code point basis, 
> instead of code unit (16bit "char") basis. Technically this change will 
> introduce a backward incompatibility, but I believe it is an 
> incompatibility to wrong behavior, not true to the meaning of those 
> methods' expectations.
> 
> Naoto
> 

Re: RFR: 8246032: Implementation of JEP 347: Adopt C++14 Language Features in HotSpot

[James Laskey]

I fully understand. I’ll set up a separate bug. 

Cheers,

— Jim



> On Jun 5, 2020, at 8:57 PM, Magnus Ihse Bursie 
>  wrote:
> 
> On 2020-06-05 13:59, Jim Laskey wrote:
>> I know there was a discussion about this elsewhere but I would like to take 
>> the opportunity to correct this now
>> 
>> make//autoconf/flags-cflags.m4:241
>> 
>>   elif test "x$TOOLCHAIN_TYPE" = xclang; then
>> if test "x$OPENJDK_TARGET_OS" = xmacosx; then
>>   # On MacOSX we optimize for size, something
>>   # we should do for all platforms?
>>   C_O_FLAG_HIGHEST_JVM="-Os"
>>   C_O_FLAG_HIGHEST="-Os"
>>   C_O_FLAG_HI="-Os"
>>   C_O_FLAG_NORM="-Os"
>>   C_O_FLAG_DEBUG_JVM=""
>> else
>>   C_O_FLAG_HIGHEST_JVM="-O3"
>>   C_O_FLAG_HIGHEST="-O3"
>>   C_O_FLAG_HI="-O3"
>>   C_O_FLAG_NORM="-O2"
>>   C_O_FLAG_DEBUG_JVM="-O0"
>> fi
>> C_O_FLAG_SIZE="-Os"
>> C_O_FLAG_DEBUG="-O0"
>> C_O_FLAG_NONE="-O0"
>>   elif test "x$TOOLCHAIN_TYPE" = xxlc; then
>> 
>> should be changed to
>> 
>>   elif test "x$TOOLCHAIN_TYPE" = xclang; then
>> C_O_FLAG_HIGHEST_JVM="-O3"
>> C_O_FLAG_HIGHEST="-O3"
>> C_O_FLAG_HI="-O3"
>> C_O_FLAG_NORM="-O2"
>> C_O_FLAG_DEBUG_JVM="-O0"
>> C_O_FLAG_SIZE="-Os"
>> C_O_FLAG_DEBUG="-O0"
>> C_O_FLAG_NONE="-O0"
>>   elif test "x$TOOLCHAIN_TYPE" = xxlc; then
>> 
>> MacOSX has been paying a historic and significant performance penalty for no 
>> valid reason.
> This might be a valid change, but it has nothing to do with C++14, and 
> changing it at the same time will increase risk for unrelated strange errors. 
> Please open a separate JBS issue for this requested change.
> 
> /Magnus
>> 
>> Otherwise +1.
>> 
>> Cheers,
>> 
>> -- Jim
>> 
>> 
>> 
 On Jun 5, 2020, at 4:52 AM, Kim Barrett  wrote:
>>> 
>>> [Changes are only to the build system, but since the changes have jdk-wide
>>> effect I’ve cc’d what I think are the relevant dev lists.]
>>> 
>>> This change is part of JEP 347; the intent is to target JDK 16.
>>> 
>>> Please review this change to the building of C++ code in the JDK to
>>> enable the use of C++14 language features.  This change does not make
>>> any code changes to use new features provided by C++11 or C++14.
>>> 
>>> This requires trimming the supported compiler versions, moving the
>>> minimum supported versions forward (significantly, in some cases).
>>> The new minimums are based on compiler documentation rather than
>>> testing.  It may be that more recent versions are actually required.
>>> 
>>> This change needs to be tested on platforms not supported by Oracle.
>>> The JEP test plan includes additional Oracle testing beyond what I’ve done.
>>> 
>>> CR:
>>> https://bugs.openjdk.java.net/browse/JDK-8246032
>>> 
>>> Webrev:
>>> https://cr.openjdk.java.net/~kbarrett/8246032/open.02/
>>> 
>>> Testing:
>>> mach5 tier1-5 on Oracle supported platforms.
>>> 
>>> Performance testing showed no significant changes in either direction.
>>> 
>>> Build-only (no tests) for linux-arm32, linux-s390x, linux-ppc64le
>>> 
> 

Re: Sometimes constraints are questionable

[James Laskey]

I’m fixing the two in java.base. The other two are in different modules and 
would require changes to export. So you can file on those. 



> On Jun 5, 2020, at 5:36 PM, Stuart Marks  wrote:
> 
> 
> 
>> On 6/3/20 10:36 AM, Stuart Marks wrote:
>> 3) Integer wraparound/overflow during size computation. AS.newLength 
>> generates this:
>> OutOfMemoryError: Required array length too large
>> (3) is the only case generated by the library. In fact, AS.hugeLength() has 
>> oldLength and minGrowth parameters, which seems like enough detail already. 
>> These could reasonably be formatted into the error message, something like:
>> private static int hugeLength(int oldLength, int minGrowth) {
>> int minLength = oldLength + minGrowth;
>> if (minLength < 0) { // overflow
>> throw new OutOfMemoryError(
>> String.format("Required array length %d + %d too large", 
>> oldLength, minGrowth));
>> }
>> Would this help? If this were added, would it be sufficient to allow various 
>> use sites to convert to use AS.newLength? (Except possibly StringJoiner.)
> 
> Anything further on this? Should I file a bug/rfe for this? Also, I could 
> update the docs to explain ArraysSupport.newLength better, per my earlier 
> exchange with David Holmes.
> 
> s'marks
> 

Re: Sometimes constraints are questionable

[James Laskey]

Understood. Just trying to balance correctness with providing meaningful 
exceptions. 

I suppose another approach is to let it all go deep and catch the error on the 
way back and provide the detail then. Not likely win any fans. 



> On May 30, 2020, at 12:30 PM, Martin Buchholz  wrote:
> 
> I wrote an earlier version of this grow logic, and then it was
> transplanted into other classes.
> 
> We strongly suspect that the VM will throw OOME when we try to
> allocate an array beyond MAX_ARRAY_SIZE, so are reluctant to do so,
> but we also consider the VM behavior a bug that may eventually get
> fixed (or is already a non-issue with a different VM).  We are trying
> for good behavior with both sorts of VM.
> 
>> On Sat, May 30, 2020 at 7:32 AM Jim Laskey  wrote:
>> 
>> I'm working through https://bugs.openjdk.java.net/browse/JDK-8230744 
>>  Several classes throw 
>> OutOfMemoryError without message .
>> 
>> I'm wondering why hugeCapacity in 
>> src/jdk.zipfs/share/classes/jdk/nio/zipfs/ByteArrayChannel.java is defined as
>> 
>>/**
>> * The maximum size of array to allocate.
>> * Some VMs reserve some header words in an array.
>> * Attempts to allocate larger arrays may result in
>> * OutOfMemoryError: Requested array size exceeds VM limit
>> */
>>private static final int MAX_ARRAY_SIZE = Integer.MAX_VALUE - 8;
>> 
>>/**
>> * Increases the capacity to ensure that it can hold at least the
>> * number of elements specified by the minimum capacity argument.
>> *
>> * @param minCapacity the desired minimum capacity
>> */
>>private void grow(int minCapacity) {
>>// overflow-conscious code
>>int oldCapacity = buf.length;
>>int newCapacity = oldCapacity << 1;
>>if (newCapacity - minCapacity < 0)
>>newCapacity = minCapacity;
>>if (newCapacity - MAX_ARRAY_SIZE > 0)
>>newCapacity = hugeCapacity(minCapacity);
>>buf = Arrays.copyOf(buf, newCapacity);
>>}
>> 
>>private static int hugeCapacity(int minCapacity) {
>>if (minCapacity < 0) // overflow
>>throw new OutOfMemoryError();
>>return (minCapacity > MAX_ARRAY_SIZE) ?
>>Integer.MAX_VALUE :
>>MAX_ARRAY_SIZE;
>>}
>> 
>> It just seems that it's pushing the inevitable off to Arrays.copyOf.  
>> Shouldn't it be:
>> 
>>private static int hugeCapacity(int minCapacity) {
>>if (minCapacity < 0 || minCapacity > MAX_ARRAY_SIZE) {
>>throw
>>new OutOfMemoryError("ByteArrayChannel exceeds maximum size: 
>> " +
>>  MAX_ARRAY_SIZE);
>>}
>> 
>>return MAX_ARRAY_SIZE;
>>}
>> 
>> Real question: is there some hidden purpose behind this kind of logic?
>> 
>> 
>> Cheers,
>> 
>> -- Jim
>> 
>> 

Re: 8246183: Scanner/ScanTest.java fails due to SIGSEGV in StubRoutines::jshort_disjoint_arraycopy

[James Laskey]

Missed that it was a backout. 



> On May 29, 2020, at 11:18 PM, James Laskey  wrote:
> 
> You may not have intended this comment. s/offset/scale/
>  49 
>  50 // Cached array base offset
>  51 private static final long ARRAY_INDEX_SCALE = 
> UNSAFE.arrayIndexScale($type$[].class);
> 
> 
> 
> 
>> On May 29, 2020, at 11:05 PM, Mikael Vidstedt  
>> wrote:
>> 
>> 
>> Looks good, thanks for fixing!
>> 
>> Cheers,
>> Mikael
>> 
>>>> On May 29, 2020, at 7:01 PM, Brian Burkhalter 
>>>>  wrote:
>>> 
>>> Please review this fix [1] for [2]. It in effect just backs out the recent 
>>> fix for [3]. I’ll investigate the root cause next week.
>>> 
>>> Thanks,
>>> 
>>> Brian
>>> 
>>> [1] http://cr.openjdk.java.net/~bpb/8246183/webrev.00/
>>> [2] https://bugs.openjdk.java.net/browse/JDK-8246183
>>> [3] https://bugs.openjdk.java.net/browse/JDK-8245121
>> 

Re: 8246183: Scanner/ScanTest.java fails due to SIGSEGV in StubRoutines::jshort_disjoint_arraycopy

[James Laskey]

You may not have intended this comment. s/offset/scale/
  49 
  50 // Cached array base offset
  51 private static final long ARRAY_INDEX_SCALE = 
UNSAFE.arrayIndexScale($type$[].class);




> On May 29, 2020, at 11:05 PM, Mikael Vidstedt  
> wrote:
> 
> 
> Looks good, thanks for fixing!
> 
> Cheers,
> Mikael
> 
>> On May 29, 2020, at 7:01 PM, Brian Burkhalter  
>> wrote:
>> 
>> Please review this fix [1] for [2]. It in effect just backs out the recent 
>> fix for [3]. I’ll investigate the root cause next week.
>> 
>> Thanks,
>> 
>> Brian
>> 
>> [1] http://cr.openjdk.java.net/~bpb/8246183/webrev.00/
>> [2] https://bugs.openjdk.java.net/browse/JDK-8246183
>> [3] https://bugs.openjdk.java.net/browse/JDK-8245121
> 

Re: RFR(xs): 8245970: IntStream.html#reduce doc should not mention average

[James Laskey]

+1



> On May 29, 2020, at 6:05 AM, Conor Cleary  wrote:
> 
> Hi,
> 
> The method-level documentation of Intstream::reduce(int, IntBinaryOperator) 
> mentions the average function as a case of reduction even though the function 
> cannot be used with the reduce method. This might cause confusion, this fix 
> therefore removes the mention of the average function from the @apiNote.
> 
> bug: https://bugs.openjdk.java.net/browse/JDK-8245970
> webrev: http://cr.openjdk.java.net/~jboes/ccleary/webrevs/8242281/webrev.00/
> 
> Regards,
> Conor
> 

Re: RFR[8245658]: 'Arrays.java has two occurrences of bad unicode constants in Javadoc.'

[James Laskey]

+1



> On May 29, 2020, at 6:37 AM, Conor Cleary  wrote:
> 
> Hi,
> 
> Could someone please review my webrev for JDK-8245658 'Arrays.java has two 
> occurrences of bad unicode constants in Javadoc.'?
> 
> In Arrays.java Javadoc, there were two instances of bad Unicode formatting 
> where the null character constant was incorrectly specified with '\u000' 
> (another zero is required). This fix displays the correct Unicode constants 
> in the Javadoc so that outputted docs display '\u'.
> 
> bug: https://bugs.openjdk.java.net/browse/JDK-8245658
> webrev: 
> http://cr.openjdk.java.net/~pconcannon/ccleary/8245658/webrevs/webrev.01/
> 
> 
> Regards,
> Conor
> 

Re: RFR 8246034: Remove java.base/share/classes/jdk/internal/jrtfs/jrtfsviewer.js and java.base/share/classes/jdk/internal/jrtfs/jrtls.js

[James Laskey]

+1



> On May 28, 2020, at 3:16 AM, sundararajan.athijegannat...@oracle.com wrote:
> 
> Please review.
> 
> Bug: https://bugs.openjdk.java.net/browse/JDK-8246034
> 
> Webrev: http://cr.openjdk.java.net/~sundar/8246034/webrev.00/
> 
> Thanks
> 
> -Sundar
> 

Re: RFR (CSR) JDK-8245399 Remove addition preview adornment from String::formatted

[James Laskey]

Thank you Paul. 



> On May 19, 2020, at 5:41 PM, Paul Sandoz  wrote:
> 
> +1
> Paul.
> 
>> On May 19, 2020, at 1:00 PM, Jim Laskey  wrote:
>> 
>> Please review this change to remove the preview heading from the javadoc of 
>> String::formatted. This also updates the @since to 15.
>> 
>> Thank you.
>> 
>> Cheers,
>> 
>> -- Jim
>> 
>> 
>> csr: https://bugs.openjdk.java.net/browse/JDK-8245399 
>> 
>> jbs: https://bugs.openjdk.java.net/browse/JDK-8245398 
>> 
>> 
>> 
> 

Re: RFR 8222100: tools/jimage/JImageTest.java time out

[James Laskey]

+1

On the road.

> On Jan 13, 2020, at 10:25 AM, sundararajan.athijegannat...@oracle.com wrote:
> 
> Bumping the default timeout (other tests in the same dir have similar 
> timeout settings).
> 
> Bug: https://bugs.openjdk.java.net/browse/JDK-8222100
> 
> Webrev: http://cr.openjdk.java.net/~sundar/8222100/webrev.00/
> 
> Thanks,
> 
> -Sundar
> 

Re: RFR: Revert: 8216553: JrtFileSystemProvider getPath(URI) omits /modules element from file path

[James Laskey]

Thank you Roger. 

Sent from my iPhone

> On May 28, 2019, at 5:36 PM, Roger Riggs  wrote:
> 
> +1,  Roger
> 
> 
>> On 05/28/2019 03:46 PM, Kim Barrett wrote:
>>> On May 28, 2019, at 3:39 PM, Jim Laskey  wrote:
>>> Please review. Need to revert a change Sundar did earlier today that is 
>>> causing 33 tier2 failures.
>>> 
>>> Cheers,
>>> 
>>> -- Jim
>>> 
>>> Reverse webrev: 
>>> http://cr.openjdk.java.net/~jlaskey/8224908/webrev/index.html 
>>> 
>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8224908 
>>> 
>>> 
>>> Job: mach5-one-jdk-13-1151-tier2-20190528-1515-2755946 
>>> 
>>> Original changset: http://hg.openjdk.java.net/jdk/jdk/rev/63ab89cc3e69 
>>> 
>> 
>> Looks good.
>> 
> 

Re: RFR: 8215017: Improve String::equals warmup characteristics

[James Laskey]

+1


> On Dec 7, 2018, at 8:11 PM, Claes Redestad  wrote:
> 
> Hi,
> 
> following up from discussions during review of JDK-8214971[1], I
> examined the startup and peak performance of a few different variant of
> writing String::equals.
> 
> Webrev: http://cr.openjdk.java.net/~redestad/8215017/jdk.00/
> Bug: https://bugs.openjdk.java.net/browse/JDK-8215017
> 
> - folding coder() == aString.coder() into sameCoder(aString) helps
> interpreter without adversely affecting higher optimization levels
> 
> - Jim's proposal to use Arrays.equals is _interesting_: it improves
> peak performance on some inputs but regress it on others. I'll defer
> that to a future RFE as it needs a more thorough examination.
> 
> - what we can do is simplify to only use StringLatin1.equals. If I'm not
> completely mistaken these are all semantically neutral (and
> StringUTF16.equals effectively redundant). If I'm completely wrong here
> I'll welcome the learning opportunity. :-)
> 
> This removes a branch and two method calls, and for UTF16 Strings we'll
> use a simpler algorithm early, which turns out to be beneficial during
> interpreter and C1 level.
> 
> I added a simple microbenchmark to explore this, results show 1.2-2.5x
> improvements in interpreter performance, while remaining perfectly
> neutral results for optimized code on this simple micro[2].
> 
> This could be extended to clean up and move StringLatin1.equals back
> into String and remove StringUTF16, but we'd also need to rearrange the
> intrinsics on the VM side. Let me know what you think.
> 
> Thanks!
> 
> /Claes
> 
> [1] 
> http://mail.openjdk.java.net/pipermail/core-libs-dev/2018-December/057162.html
> 
> [2]
> == Baseline =
> 
> -Xint
> BenchmarkMode  Cnt ScoreError  Units
> StringEquals.equalsAlmostEqual   avgt4   968.640 ±  1.337  ns/op
> StringEquals.equalsAlmostEqualUTF16  avgt4  2082.007 ±  5.303  ns/op
> StringEquals.equalsDifferent avgt4   583.166 ± 29.461  ns/op
> StringEquals.equalsDifferentCoders   avgt4   422.993 ±  1.291  ns/op
> StringEquals.equalsEqual avgt4   988.671 ±  1.492  ns/op
> StringEquals.equalsEqualsUTF16   avgt4  2103.060 ±  5.705  ns/op
> 
> -XX:+CompactStrings
> BenchmarkMode  Cnt   Score   Error  Units
> StringEquals.equalsAlmostEqual   avgt4  23.896 ± 0.089  ns/op
> StringEquals.equalsAlmostEqualUTF16  avgt4  23.935 ± 0.562  ns/op
> StringEquals.equalsDifferent avgt4  15.086 ± 0.044  ns/op
> StringEquals.equalsDifferentCoders   avgt4  12.572 ± 0.008  ns/op
> StringEquals.equalsEqual avgt4  25.143 ± 0.025  ns/op
> StringEquals.equalsEqualsUTF16   avgt4  25.148 ± 0.021  ns/op
> 
> -XX:-CompactStrings
> BenchmarkMode  Cnt   Score   Error  Units
> StringEquals.equalsAlmostEqual   avgt4  24.539 ± 0.127  ns/op
> StringEquals.equalsAlmostEqualUTF16  avgt4  22.638 ± 0.047  ns/op
> StringEquals.equalsDifferent avgt4  13.930 ± 0.835  ns/op
> StringEquals.equalsDifferentCoders   avgt4  13.836 ± 0.025  ns/op
> StringEquals.equalsEqual avgt4  26.420 ± 0.020  ns/op
> StringEquals.equalsEqualsUTF16   avgt4  23.889 ± 0.037  ns/op
> 
> == Fix ==
> 
> -Xint
> BenchmarkMode  CntScore Error  Units
> StringEquals.equalsAlmostEqual   avgt4  811.859 ±   8.663  ns/op
> StringEquals.equalsAlmostEqualUTF16  avgt4  802.784 ± 352.884  ns/op
> StringEquals.equalsDifferent avgt4  431.837 ±   1.884  ns/op
> StringEquals.equalsDifferentCoders   avgt4  358.244 ±   1.208  ns/op
> StringEquals.equalsEqual avgt4  832.056 ±   3.541  ns/op
> StringEquals.equalsEqualsUTF16   avgt4  832.434 ±   3.516  ns/op
> 
> -XX:+CompactStrings
> BenchmarkMode  Cnt   Score   Error  Units
> StringEquals.equalsAlmostEqual   avgt4  23.906 ± 0.151  ns/op
> StringEquals.equalsAlmostEqualUTF16  avgt4  23.905 ± 0.123  ns/op
> StringEquals.equalsDifferent avgt4  15.088 ± 0.023  ns/op
> StringEquals.equalsDifferentCoders   avgt4  12.575 ± 0.030  ns/op
> StringEquals.equalsEqual avgt4  25.149 ± 0.059  ns/op
> StringEquals.equalsEqualsUTF16   avgt4  25.149 ± 0.033  ns/op
> 
> -XX:-CompactStrings
> BenchmarkMode  Cnt   Score   Error  Units
> StringEquals.equalsAlmostEqual   avgt4  24.521 ± 0.050  ns/op
> StringEquals.equalsAlmostEqualUTF16  avgt4  22.639 ± 0.035  ns/op
> StringEquals.equalsDifferent avgt4  13.831 ± 0.020  ns/op
> StringEquals.equalsDifferentCoders   avgt4  13.884 ± 0.345  ns/op
> StringEquals.equalsEqual avgt4  26.395 ± 0.066  ns/op
> StringEquals.equalsEqualsUTF16   avgt4  23.904 ± 0.112  ns/op

Re: RFR - JDK-8215493 String::indent inconsistency with blank lines

[James Laskey]

It’s there.  AlignIndent.java. 

Sent from my iPhone

> On Dec 20, 2018, at 12:10 PM, Roger Riggs  wrote:
> 
> Hi Jim,
> 
> Is there a missing test?  I'd expect to see a corresponding test change also.
> 
> otherwise, looks fine.
> 
> Regards, Roger
> 
> 
>> On 12/19/2018 03:13 PM, Jim Laskey wrote:
>> Please review these changes for jdk 12.
>> 
>> webrev: http://cr.openjdk.java.net/~jlaskey/8215493/webrev/index.html 
>> 
>> 
>> There is inconsistency with regards to blank lines and indentation. Adding 
>> indent space ignores blank lines, where subtraction indentation white space 
>> does not ignore blank lines. The solution is to remove the condition "Blank 
>> lines are unaffected" from the specification with appropriate changes to the 
>> implementation.
>> 
>> Thank you.
>> 
>> Cheers,
>> 
>> — Jim
>> 
>> CSR: https://bugs.openjdk.java.net/browse/JDK-8215499 
>> 
>> JBS: https://bugs.openjdk.java.net/browse/JDK-8215493 
>> 
>> 
>> 
> 

Re: RFR 8214971 : Replace use of string.equals("") with isEmpty()

[James Laskey]

Or simply;

if (anObject instanceof String) {
   String aString = (String)anObject;
   if (coder() == aString.coder())
return Arrays.equals(value, aString.value);
   }
   }

Sent from my iPhone

> On Dec 6, 2018, at 10:56 PM, Stuart Marks  wrote:
> 
>> On 12/6/18 2:42 PM, Claes Redestad wrote:
>> I filed this bug after seeing it in startup profiles, where isEmpty() avoids 
>> an instanceof and four(!) method calls, so getting rid of a few of these has 
>> a measurable impact on startup. It seemed prudent to just replace it all 
>> while we're at it.
> 
> Interesting. The compact strings stuff saves a lot of space, but it means 
> that more setup work needs to be done before an actual equality comparison 
> can be done. Thus, equals("") has gotten quite a bit more expensive relative 
> to isEmpty() compared with the situation prior to compact strings.
> 
> Still, it seems like a method call could be saved here:
> 
>if (anObject instanceof String) {
>String aString = (String)anObject;
>if (coder() == aString.coder()) {
>return isLatin1() ? StringLatin1.equals(value, aString.value)
>  : StringUTF16.equals(value, aString.value);
>}
>}
> 
> by saving the result of coder() in a local variable and then comparing it 
> directly to LATIN1. Is it worth it?
> 
> (Note, this isn't relevant to the current review.)
> 
> s'marks

Re: RFR 8171426 : java/lang/ProcessBuilder/Basic.java failed with Stream closed

[James Laskey]

I wasn’t as fussy about the original comment but this one is also fine. 

Sent from my iPhone

> On Dec 4, 2018, at 12:58 PM, Roger Riggs  wrote:
> 
> Hi Brian, Jim,
> 
> Thanks for the reviews.
> 
> I simplify the description, the full details are in the bug report
> and are more complete.
> 
> Updated webrev:
>   http://cr.openjdk.java.net/~rriggs/webrev-basic-8171426-1/
> 
> Thanks, Roger
> 
> 
>> On 12/03/2018 04:49 PM, Brian Burkhalter wrote:
>> Hi Roger,
>> 
>> L2:copyright year, of course
>> L2119: The verbiage seems a little obtuse.
>> 
>> Otherwise, +1.
>> 
>> Thanks,
>> 
>> Brian
>> 
>>> On 11/27/2018 02:04 PM, Roger Riggs wrote:
>>> Please review a test update to address an intermittent test failure.
>>> The test is modified to accept the current implementation behavior of the 
>>> input streams
>>> from processes that may under race conditions with close throw IOE("Stream 
>>> closed").
>>> 
>>> Details and analysis in the Jira:
>>> https://bugs.openjdk.java.net/browse/JDK-8171426
>>> 
>>> Webrev:
>>> http://cr.openjdk.java.net/~rriggs/webrev-basic-8171426/ 
>>> 
>>> 
>>> Thanks, Roger
>> 
> 

Re: RFR: 8205438: Re-enable shebang tests in test/jdk/tools/launchers/SourceMode.java

[James Laskey]

We went through the same exercise with jjs. Tough coming up with shebang tests 
that worked on all platforms. Shebang args vs command line args was a 
nightmare. 

Sent from my iPhone

> On Jun 27, 2018, at 6:02 PM, mandy chung  wrote:
> 
> Looks good.  It's amazing to find out 3 different behavior on these 3 
> platforms.
> 
> Mandy
> 
>> On 6/27/18 1:37 PM, Jonathan Gibbons wrote:
>> Please review a test fix to re-enable shebang tests in 
>> test/jdk/tools/launchers/SourceMode.java .
>> The test cases for invoking the source launcher via the shebang mechanism 
>> have been disabled because they were adversely affected by the very long 
>> pathnames on our internal test infastructure, causing the test cases to 
>> overflow the system limit of 128 characters for the first line of a shebang 
>> file
>> The fix is to run jlink to create a small image that can be addressed by a 
>> short relative path.
>> Some of the test cases on macOS and Solaris remain explicitly disabled 
>> because of the inconsistencies in the system-level support for shebang 
>> execution.
>> Finally, the use of hard-coded version numbers in the test is changed to 
>> avoid having to add this test to the list of tests that need to be modified 
>> at the beginning of each release.
>> -- Jon
>> JBS: https://bugs.openjdk.java.net/browse/JDK-8205438
>> Webrev: http://cr.openjdk.java.net/~jjg/8205438/webrev.00/
> 
> 

Re: BiCollector

[James Laskey]

Replicator (as in DNA)

Sent from my iPhone

> On Jun 18, 2018, at 6:49 PM, James Laskey  wrote:
> 
> Bifurcate 
> 
> Sent from my iPhone
> 
>> On Jun 18, 2018, at 6:29 PM, Brian Goetz  wrote:
>> 
>> "bisecting" sounds like it sends half the elements to one collector and half 
>> to the other ...
>> 
>> "tee" might be a candidate, though it doesn't follow the `ing convention.  
>> "teeing" sounds dumb.
>> 
>> 
>> 
>>> On 6/15/2018 7:36 PM, Paul Sandoz wrote:
>>> Hi Tagir,
>>> 
>>> This is looking good.
>>> 
>>> My current favorite name for the factory method is “bisecting” since we are 
>>> dividing the collector into two collectors, the results of which are then 
>>> merged.
>>> Suggested first paragraph of the factory method:
>>> 
>>>  "Returns a collector that passes the input elements to two specified 
>>> collectors and merges their results with the specified merge function.”
>>> 
>>> Paul.
>>> 
>>>> On Jun 15, 2018, at 4:26 AM, Tagir Valeev  wrote:
>>>> 
>>>> Hello!
>>>> 
>>>> I created a preliminary webrev of my own implementation (no testcases yet):
>>>> http://cr.openjdk.java.net/~tvaleev/patches/pairing/webrev/
>>>> If anybody wants to sponsor my implementation, I will happily log an issue 
>>>> and write tests.
>>>> 
>>>> The name "pairing" was invented by me, but as I'm not a native English 
>>>> speaker I cannot judge whether it's optimal, so better ideas are welcome.
>>>> Also I decided to remove accumulator types from public type variables. 
>>>> They do not add anything to type signature, only clutter it
>>>> increasing the number of type parameters from 4 to 6. I think it was a 
>>>> mistake to expose the accumulator type parameter in other cascading 
>>>> collectors
>>>> like filtering(), collectingAndThen(), groupingBy(), etc. I'm not 
>>>> insisting though, if you feel that conformance to existing collectors is
>>>> more important than simplicity.
>>>> 
>>>> With best regards,
>>>> Tagir Valeev.
>>>> 
>>>>> On Fri, Jun 15, 2018 at 5:05 AM Brian Goetz  
>>>>> wrote:
>>>>> 
>>>>> Well, I don't see the need to pack the two results into a Map.Entry
>>>>> (or any similar) container as a drawback.
>>>> From an "integrity of the JDK APIs" perspective, it is unquestionably a
>>>> drawback.  These items are not a Key and an associated Value in a Map;
>>>> it's merely pretending that Map.Entry really means "Pair".  There's a
>>>> reason we don't have a Pair class in the JDK (and no, let's not reopen
>>>> that now); using something else as a Pair proxy that is supposed to have
>>>> specific semantics is worse. (It's fine to do this in your own code, but
>>>> not in the JDK. Different standards for code that has different audiences.)
>>>> 
>>>> Tagir's proposed sidestepping is nice, and it will also play nicely with
>>>> records, because then you can say:
>>>> 
>>>>  record NameAndCount(String name, int count);
>>>> 
>>>>  stream.collect(pairing(collectName, collectCount, NameAndCount::new));
>>>> 
>>>> and get a more properly abstract result out.  And its more in the spirit
>>>> of existing Collectors.  If you want to use Map.Entry as an
>>>> _implementation detail_, that's fine.
>>>> 
>>>> I can support this form.
>>>> 
>>>>> I also don't see a larger abstraction like BiStream as a natural fit
>>>>> for a similar thing.
>>>> I think the BiStream connection is mostly tangential.  We tried hard to
>>>> support streams of (K,V) pairs when we did streams, as Paul can attest,
>>>> but it was a huge complexity-inflater and dropping this out paid an
>>>> enormous simplification payoff.
>>>> 
>>>> With records, having streams of tuples will be simpler to represent, but
>>>> no more performant; it will take until we get to value types and
>>>> specialized generics to get the performance we want out of this.
>>>> 
>>>> 
>> 
> 

Re: BiCollector

[James Laskey]

Bifurcate 

Sent from my iPhone

> On Jun 18, 2018, at 6:29 PM, Brian Goetz  wrote:
> 
> "bisecting" sounds like it sends half the elements to one collector and half 
> to the other ...
> 
> "tee" might be a candidate, though it doesn't follow the `ing convention.  
> "teeing" sounds dumb.
> 
> 
> 
>> On 6/15/2018 7:36 PM, Paul Sandoz wrote:
>> Hi Tagir,
>> 
>> This is looking good.
>> 
>> My current favorite name for the factory method is “bisecting” since we are 
>> dividing the collector into two collectors, the results of which are then 
>> merged.
>>  Suggested first paragraph of the factory method:
>> 
>>   "Returns a collector that passes the input elements to two specified 
>> collectors and merges their results with the specified merge function.”
>> 
>> Paul.
>>  
>>> On Jun 15, 2018, at 4:26 AM, Tagir Valeev  wrote:
>>> 
>>> Hello!
>>> 
>>> I created a preliminary webrev of my own implementation (no testcases yet):
>>> http://cr.openjdk.java.net/~tvaleev/patches/pairing/webrev/
>>> If anybody wants to sponsor my implementation, I will happily log an issue 
>>> and write tests.
>>> 
>>> The name "pairing" was invented by me, but as I'm not a native English 
>>> speaker I cannot judge whether it's optimal, so better ideas are welcome.
>>> Also I decided to remove accumulator types from public type variables. They 
>>> do not add anything to type signature, only clutter it
>>> increasing the number of type parameters from 4 to 6. I think it was a 
>>> mistake to expose the accumulator type parameter in other cascading 
>>> collectors
>>> like filtering(), collectingAndThen(), groupingBy(), etc. I'm not insisting 
>>> though, if you feel that conformance to existing collectors is
>>> more important than simplicity.
>>> 
>>> With best regards,
>>> Tagir Valeev.
>>> 
 On Fri, Jun 15, 2018 at 5:05 AM Brian Goetz  wrote:
 
 Well, I don't see the need to pack the two results into a Map.Entry
 (or any similar) container as a drawback.
>>>  From an "integrity of the JDK APIs" perspective, it is unquestionably a
>>> drawback.  These items are not a Key and an associated Value in a Map;
>>> it's merely pretending that Map.Entry really means "Pair".  There's a
>>> reason we don't have a Pair class in the JDK (and no, let's not reopen
>>> that now); using something else as a Pair proxy that is supposed to have
>>> specific semantics is worse. (It's fine to do this in your own code, but
>>> not in the JDK. Different standards for code that has different audiences.)
>>> 
>>> Tagir's proposed sidestepping is nice, and it will also play nicely with
>>> records, because then you can say:
>>> 
>>>   record NameAndCount(String name, int count);
>>> 
>>>   stream.collect(pairing(collectName, collectCount, NameAndCount::new));
>>> 
>>> and get a more properly abstract result out.  And its more in the spirit
>>> of existing Collectors.  If you want to use Map.Entry as an
>>> _implementation detail_, that's fine.
>>> 
>>> I can support this form.
>>> 
 I also don't see a larger abstraction like BiStream as a natural fit
 for a similar thing.
>>> I think the BiStream connection is mostly tangential.  We tried hard to
>>> support streams of (K,V) pairs when we did streams, as Paul can attest,
>>> but it was a huge complexity-inflater and dropping this out paid an
>>> enormous simplification payoff.
>>> 
>>> With records, having streams of tuples will be simpler to represent, but
>>> no more performant; it will take until we get to value types and
>>> specialized generics to get the performance we want out of this.
>>> 
>>> 
> 

Re: RFR JDK-8200172,String.split non-positive term incorrect use

[James Laskey]

+1

Sent from my iPhone

> On May 22, 2018, at 9:43 PM, Xueming Shen  wrote:
> 
> Thanks!
> 
> webrev has been updated as suggested.
> 
> http://cr.openjdk.java.net/~sherman/8200172/webrev/
> 
> -Sherman
> 
>> On 5/22/18, 4:30 PM, joe darcy wrote:
>> Hello,
>> 
>> I think some larger re-wording is in order. Here is one of the proposed new 
>> paragraphs:
>> 
>> 2181  *  The {@code limit} parameter controls the number of times the
>> 2182  * pattern is applied and therefore affects the length of the 
>> resulting
>> 2183  * array.  If the limit n is greater than zero then the 
>> pattern
>> 2184  * will be applied at most n-1 times, the array's
>> 2185  * length will be no greater than n, and the array's last 
>> entry
>> 2186  * will contain all input beyond the last matched delimiter.  If 
>> n
>> 2187  * is negative then the pattern will be applied as many times as
>> 2188  * possible and the array can have any length.  If n is zero 
>> then
>> 2189  * the pattern will be applied as many times as possible, the array 
>> can
>> 2190  * have any length, and trailing empty strings will be discarded.
>> 
>> In a mathematical signed-ness sense there are three values, positive, zero, 
>> and negative, hence library methods like Integer.signum which return -1, 0, 
>> or 1. The term non-negative covers zero and positive values; conversely 
>> non-positive covers zero and negative.
>> 
>> In terms of how the above paragraph could be structured, I'd recommend
>> 
>> "If the limit n is positive...
>> If the limit n is zero...
>> if the limit n is negative..."
>> 
>> possibly using an unordered list.
>> 
>> No CSR would be required for this kind of change as the semantics of the 
>> specification is not being altered.
>> 
>> HTH,
>> 
>> -Joe
>> 
>> 
>>> On 5/22/2018 4:13 PM, Lance Andersen wrote:
>>> Hi Sherman
>>> 
>>> The change from non-positive to negative makes sense.
>>> 
>>> I would agree that a CSR should not be required (hopefully Joe D does also 
>>> ;-) )
>>> 
>>> Best
>>> Lance
 On May 22, 2018, at 7:07 PM, Xueming Shen  wrote:
 
 Hi,
 
 Please help review a api doc clarification for 
 String.split()/Pattern.split().
 
 issue: https://bugs.openjdk.java.net/browse/JDK-8200172
 webrev: http://cr.openjdk.java.net/~sherman/8200172/webrev
 
 As suggested, it appears to be clear, straightforward and less confusion 
 to simply
 categorize the clauses as "if positive", "if negative" and "if zero".
 
 It's simply a rewording to clear things up, I would assume csr is not 
 necessary here.
 
 thanks,
 Sherman
 
>>> 
>>>  
>>> 
>>> Lance Andersen| 
>>> Principal Member of Technical Staff | +1.781.442.2037
>>> Oracle Java Engineering
>>> 1 Network Drive
>>> Burlington, MA 01803
>>> lance.ander...@oracle.com 
>>> 
>>> 
>>> 
>> 
> 

Re: RFR JDK-8200172,String.split non-positive term incorrect use

[James Laskey]

+1 

Thinking still needs CSR to track the change. 

Sent from my iPhone

> On May 22, 2018, at 8:07 PM, Xueming Shen  wrote:
> 
> Hi,
> 
> Please help review a api doc clarification for String.split()/Pattern.split().
> 
> issue: https://bugs.openjdk.java.net/browse/JDK-8200172
> webrev: http://cr.openjdk.java.net/~sherman/8200172/webrev
> 
> As suggested, it appears to be clear, straightforward and less confusion to 
> simply
> categorize the clauses as "if positive", "if negative" and "if zero".
> 
> It's simply a rewording to clear things up, I would assume csr is not 
> necessary here.
> 
> thanks,
> Sherman
> 

Re: RFR JDK-8200372 - String::trim JavaDoc should clarify meaning of space

[James Laskey]

Will do. 

Sent from my iPhone

> On May 9, 2018, at 10:38 PM, Stuart Marks  wrote:
> 
> A typical way to refer to a particular Unicode character by code point hex 
> value is U+ (with more x's if necessary). For example,
> 
> 2602  * Returns a string whose value is this string, with all leading
> 2603  * and trailing space removed, where space is defined
> 2604  * as any character whose codepoint is less than or equal to
> 2605  * U+0020 (the space character).
> 
> It doesn't even need to be the code font.
> 
> Oh well, you pushed already. Maybe fix this up in your next change to 
> String.java.
> 
> s'marks
> 
> 
> 
>> On 5/8/18 6:43 AM, Roger Riggs wrote:
>> Hi Jim,
>> I would agree about code points in methods that refer to code points and 
>> need a more
>> precise notation.  However, trim() is not one of them and the alternative 0x 
>> format is quite acceptable.
>> Would the syntax for raw string literals (not there yet) make the source 
>> more readable?
>> Roger
>>> On 5/8/2018 9:36 AM, Jim Laskey wrote:
>>> Roger,
>>> 
>>> You withdrew the comment from the CSR so I assumed that you had changed 
>>> your mind.
>>> 
>>> Stuart, Sherman and Joe have be pushing the use of codepoints versus char 
>>> (or ASCII) in new character related comments hence the choice of ‘\u' 
>>> notation. Unfortunately, unicode preprocessing vs backslash processing vs 
>>> Javadoc does not allow the '\\u0020' in comments (it ends up being 
>>> '\\u0020’ in the Javadoc) and '\u0020’ just ends up being ‘ ‘.
>>> 
>>> Cheers,
>>> 
>>> — Jim
>>> 
>>> 
>>> 
>>> 
 On May 8, 2018, at 10:04 AM, Roger Riggs  wrote:
 
 Hi Jim,
 
 The use of \u005c in the source makes the source code unreadable.
 The more conventional use of the 0x prefix (i.e. 0x0130) is preferred.
 Though \u is necessary in some cases, it should be avoided where a more 
 readable alternative is available.
 
 Thanks, Roger
 
 
> On 5/8/2018 8:19 AM, Jim Laskey wrote:
> Comment change approved in CSR
> 
> webrev: http://cr.openjdk.java.net/~jlaskey/8200372/webrev/index.html
> JBS: https://bugs.openjdk.java.net/browse/JDK-8200372
> CSR: https://bugs.openjdk.java.net/browse/JDK-8196005

Re: RFR: JDK-8197594: String#repeat

[James Laskey]

Thanks Stuart. RE apinote, I was trying to follow the pattern of other older 
method comments (Roman-style.) Comments/Javadoc in most of these older classes 
are a mix of styles. Question: if you update/clean-up a method in an older 
class, should you bring the comment/Javadoc up-to-date as well?

Cheers,

— Jim

Sent from my iPhone

> On Feb 28, 2018, at 6:21 PM, Stuart Marks  wrote:
> 
> Hi Jim,
> 
> Implementation looks good. I'd suggest a couple small editorial changes to 
> the spec:
> 
> 2966 /**
> 2967  * Returns a string whose value is the concatenation of this
> 2968  * string repeated {@code count} times.
> 2969  * 
> 2970  * If count or length is zero then the empty string is returned.
> 
> I went looking for a 'length' parameter, until I realized it means the length 
> of *this string*. So maybe clarify that. (And also line 2977.)
> 
> 2971  * 
> 2972  * This method may be used to create space padding for
> 2973  * formatting text or zero padding for formatting numbers.
> 
> I don't think having this text is necessary, but if you want it, I'd suggest 
> putting it into an @apiNote.
> 
> 2974  * @param   count number of times to repeat
> 2975  * @return  A string composed of this string repeated
> 2976  *  {@code count} times or the empty string if count
> 2977  *  or length is zero.
> 
> Thanks,
> 
> s'marks
> 
> 
>> On 2/28/18 8:31 AM, Jim Laskey wrote:
>> Introduction of a new instance method String::repeat to allow an efficient 
>> and concise approach for generating repeated character sequences as strings.
>> Performance information in JBS.
>> Thank you.
>> Cheers,
>> — Jim
>> JBS: https://bugs.openjdk.java.net/browse/JDK-8197594 
>> 
>> CSR: https://bugs.openjdk.java.net/browse/JDK-8198296
>>  
>> Webrev: http://cr.openjdk.java.net/~jlaskey/8197594/webrev-02/index.html 
>> 
>> JavaDoc: http://cr.openjdk.java.net/~jlaskey/8197594/String.html 
>> 

Re: RFR: JDK-8197594: String#repeat

[James Laskey]

Thanks Paul. 

Sent from my iPhone

> On Feb 28, 2018, at 10:13 PM, Paul Sandoz  wrote:
> 
> Hi Jim,
> 
> Looks good. I like the power of 2 copying.
> 
> 
> 2978  * @throws  IllegalArgumentException if the {@code count} is
> 2979  *  negative.
> 2980  */
> 2981 public String repeat(int count) {
> 
> Missing @since11 on the method.
> 
> 
> Like Stuart suggests, turn the explanatory text into an api note, perhaps 
> with a small code sample.
> 
> Thanks,
> Paul.
> 
> 
>> On Feb 28, 2018, at 8:31 AM, Jim Laskey  wrote:
>> 
>> Introduction of a new instance method String::repeat to allow an efficient 
>> and concise approach for generating repeated character sequences as strings.
>> 
>> Performance information in JBS.
>> 
>> Thank you.
>> 
>> Cheers,
>> 
>> — Jim
>> 
>> 
>> JBS: https://bugs.openjdk.java.net/browse/JDK-8197594 
>> 
>> CSR: https://bugs.openjdk.java.net/browse/JDK-8198296
>> 
>> Webrev: http://cr.openjdk.java.net/~jlaskey/8197594/webrev-02/index.html 
>> 
>> JavaDoc: http://cr.openjdk.java.net/~jlaskey/8197594/String.html 
>> 
>> 
>> 
>> 
> 

Re: RFR: 8197594 - String and character repeat

[James Laskey]

Didn’t I hear someone mentioning “\U1D11A” at some point?

Sent from my iPhone

> On Feb 18, 2018, at 1:10 AM, Stuart Marks  wrote:
> 
> 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)
> 
> But this is still fairly roundabout. Since most cases are constants, the 
> advice is to use a string literal instead of a char literal. This works for 
> BMP characters, e.g. "-".repeat(10) or "\u2501".repeat(15). But if I want a 
> non-BMP character as a string literal, I have encode it into a surrogate pair 
> myself. For example, a string literal containing the character U+1D11A 
> MUSICAL SYMBOL FIVE-LINE STAFF would be "\uD834\uDD1A". Ugh! Or, I could just 
> call a function and live with it not being a constant. It would be nice if 
> there were an escape sequence that allowed any Unicode code point, including 
> supplementary characters, to be put to n a string literal.
> 
> s'marks
> 
>> On Feb 16, 2018, at 18:02, Brian Goetz  wrote:
>> 
>> Disagree.  
>> 
>> On #3, most of the time the char being repeated is already a literal.  So 
>> just make it a string.  
>> 
>> On #2, better to aim for string.ofCodePoint(int) and compose w repeat.  
>> 
>> Down to one method again :)
>> 
>> Sent from my MacBook Wheel
>> 
>>> On Feb 16, 2018, at 5:13 PM, Stuart Marks  wrote:
>>> 
>>> Let me put in an argument for handling code points:
>>> 
 3. public static String repeat(final int codepoint, final int count)
>>> 
>>> Most of the String and Character API handles code points on an equal 
>>> footing with chars. I think this is important, as over time Unicode is 
>>> continuing to add supplementary characters -- those that can't be 
>>> represented in a Java char value. Examples abound of how such characters 
>>> are mishandled. Therefore, I believe Java APIs should have full support for 
>>> code points.
>>> 
>>> This is a small thing, and some might consider it a rare case -- how often 
>>> does one need to repeat something like an emoji? The issue however isn't 
>>> that particular use case. Instead what's required is the ability to handle 
>>> *any Unicode character* uniformly, regardless of whether or not it's a 
>>> supplementary character. The way to do that is to deal with code points, so 
>>> any Java API that deals with character data must also handle code points.
>>> 
>>> If we were to add just one method:
>>> 
 1. public String repeat(final int count)
>>> 
>>> the workaround is to take the character, turn it into a string, and call 
>>> the repeat() method on it. For a 'char' value, this isn't too bad, but I'd 
>>> argue it isn't pretty either:
>>> 
>>>  Character.toString(charVal).repeat(n)
>>> 
>>> But this only handles BMP characters, not supplementary characters. 
>>> Unfortunately, there's no direct way to turn a code point into a string -- 
>>> you have to turn it into a byte array first! Thus, to get a string from a 
>>> code point and repeat it, you have to do this:
>>> 
>>>  new String(Character.toChars(codepoint)).repeat(count)
>>> 
>>> This is enough indirection that it's hard to discover, and I suspect that 
>>> most people won't put in the effort to do this correctly, resulting in more 
>>> code that mishandles supplementary characters.
>>> 
>>> Thus, I think we need to add API #3 that performs the repeat function on 
>>> code points.
>>> 
>>> (Hm, the lack of Character.toString(codepoint) is covered by JDK-4993841, 
>>> which is closed. I think I'll reopen it.)
>>> 
 2. public static String repeat(final char ch, final int count)
>>> 
>>> I can see that this API is not as important as one that handles code 
>>> points, and it seems to be less frequently used according to Louis W's 
>>> analysis. But if you have char data you want to repeat, not having this 
>>> seems like an omission; it seems backwards to have to create a string from 
>>> the char, only for repeat() to extract that char from that String in order 
>>> to repeat it. Thus I've vote for inclusion of this method as well.
>>> 
>>> s'marks
>>> 
>>> 
 On 2/16/18 5:10 AM, Jim Laskey wrote:
 We’re going with the one instance method (Louis clinched it.) with 
 recommended enhancements and not touching CharSequence.
 Working it up now.
 — Jim
> On Feb 16, 2018, at 7:46 AM, Alan Bateman  wrote:
> 
> On 15/02/2018 17:20, Jim Laskey wrote:
>> This is a pre-CSR code review [1] for String repeat methods 
>> (Enhancement).
>> 
>> The proposal is to introduce four new methods;
>> 
>> 1. public String repeat(final int count)
>> 2. public static String repeat(final char ch, final int count)
>> 3. public static String repeat(final int codepoint, final int count)
>> 

Re: RFR: 8197594 - String and character repeat

[James Laskey]

Good to have the data. Thank you Louis. 

Sent from my iPhone

> On Feb 15, 2018, at 4:52 PM, Louis Wasserman  wrote:
> 
> I don't think there's a case for demand to merit having a 
> repeat(CharSequence, int) at all.  
> 
> I did an analysis of usages of Guava's Strings.repeat on Google's codebase.  
> Users might be rolling their own implementations, too, but this should be a 
> very good proxy for demand.
> 
> StringRepeat_SingleConstantChar   = 4.475 K // strings with .length() ==  
> 1
> StringRepeat_SingleConstantCodePoint  = 28 // strings with 
> .codePointCount(...) == 1
> StringRepeat_MultiCodePointConstant   = 1.156 K // constant strings neither 
> of the above
> StringRepeat_CharSequenceToString = 2 // 
> Strings.repeat(CharSequence.toString(), n)
> StringRepeat_NoneOfTheAbove   = 248
> 
> Notably, it seems like basically nobody needs to repeat a CharSequence -- 
> definitely not enough demand to merit the awkwardness of e.g. Rope.repeat(n) 
> inheriting a repeat returning a String.
> 
> Based on this data, I'd recommend providing one and only one method of this 
> type: String.repeat(int).  There's no real advantage to a static repeat(char, 
> int) method when the overwhelming majority of these are constants: e.g. 
> compare SomeUtilClass.repeat('*', n) versus "*".repeat(n).  
> Character.toString(c).repeat(n) isn't a bad workaround if you don't have a 
> constant char.  There also isn't much demand for dealing with the code point 
> case specially; the String.repeat(int) method seems like it'd handle that 
> just fine.
> 
>> On Thu, Feb 15, 2018 at 11:44 AM Jim Laskey  wrote:
>> 
>> 
>> > On Feb 15, 2018, at 3:36 PM, Ivan Gerasimov  
>> > wrote:
>> >
>> > Hello!
>> >
>> > The link with the webrev returned 404, but I could find it at this 
>> > location: http://cr.openjdk.java.net/~jlaskey/8197594/webrev-00/
>> >
>> > A few minor comments:
>> >
>> > 1)
>> >
>> > This check:
>> >
>> > 2992 final long limit = (long)count * 2L;
>> > 2993 if ((long)Integer.MAX_VALUE < limit) {
>> >
>> > can be possibly simplified as
>> > if (count > Integer.MAX_VALUE - count) {
>> 
>> Good.
>> 
>> >
>> > 2)
>> > Should String repeat(final int codepoint, final int count) be optimized 
>> > for codepoints that can be represented with a single char?
>> >
>> > E.g. like this:
>> >
>> > public static String repeat(final int codepoint, final int count) {
>> >return Character.isBmpCodePoint(codepoint))
>> >? repeat((char) codepoint, count)
>> >: (new String(Character.toChars(codepoint))).repeat(count);
>> > }
>> 
>> Yes, avoid array allocation.
>> 
>> >
>> > 3)
>> > Using long arithmetic can possibly be avoided in the common path of 
>> > repeat(final int count):
>> >
>> > E.g. like this:
>> >
>> > if (count < 0) {
>> > throw new IllegalArgumentException("count is negative, " + 
>> > count);
>> > } else if (count == 1) {
>> > return this;
>> > } else if (count == 0) {
>> > return "";
>> > }
>> > final int len = value.length;
>> > if (Integer.MAX_VALUE / count < len) {
>> > throw new IllegalArgumentException(
>> > "Resulting string exceeds maximum string length: " + 
>> > ((long)len * (long)count));
>> > }
>> > final int limit = count * len;
>> 
>> Good.
>> 
>> Thank you.
>> 
>> >
>> > With kind regards,
>> > Ivan
>> >
>> > On 2/15/18 9:20 AM, Jim Laskey wrote:
>> >> This is a pre-CSR code review [1] for String repeat methods (Enhancement).
>> >>
>> >> The proposal is to introduce four new methods;
>> >>
>> >> 1. public String repeat(final int count)
>> >> 2. public static String repeat(final char ch, final int count)
>> >> 3. public static String repeat(final int codepoint, final int count)
>> >> 4. public static String repeat(final CharSequence seq, final int count)
>> >>
>> >> For the sake of transparency, only 1 is necessary, 2-4 are convenience 
>> >> methods.
>> >> In the case of 2, “*”.repeat(10) performs as well as String.repeat(‘*’, 
>> >> 10).
>> >> 3 and 4 convert to String before calling 1.
>> >>
>> >> Performance runs with jmh (results as comment in [2]) show that these
>> >> methods are significantly faster that StringBuilder equivalents.
>> >>  - fewer memory allocations
>> >>  - fewer char to byte array conversions
>> >>  - faster pyramid replication vs O(N) copying
>> >>
>> >> I left StringBuilder out of scope. It falls under the category of
>> >> Appendables#append with repeat. A much bigger project.
>> >>
>> >> All comments welcome. Especially around the need for convenience
>> >> methods, the JavaDoc content and expanding the tests.
>> >>
>> >> — Jim
>> >>
>> >> [1] webrev: http://cr.openjdk.java.net//oj/home/jlaskey/8197594/webrev-00
>> >> [2] jbs: https://bugs.openjdk.java.net/browse/JDK-8197594
>> >>
>> >>
>> >
>> > --
>> > With kind regards,
>> > Ivan 

Re: RFR: JDK-8161067 - jlink: Enable plugins to use the module pool for class lookup

[James Laskey]

Sent from my iPhone

> On Jul 11, 2016, at 11:09 AM, Paul Sandoz  wrote:
> 
> 
>> On 11 Jul 2016, at 14:17, Jim Laskey (Oracle)  
>> wrote:
>> 
>> I’m not sure if we can determine supplied classes vs others unless we 
>> provide a flag or the set of supplied modules.  At any rate, the rules for 
>> cross module optimization would still be complex.
> 
> Ok.
> 
> 
>> ModuleEntry.findEntry: I was half thinking the same once I realized the 
>> limits of the lookup.  The complication is that a ModuleEntry may exist in 
>> several pools (direct copy).  So, though we have to create new LinkModules 
>> each pass, we would also be forced to create new ModuleEntrys for each pass. 
>>  In practice, I’m not sure findEntry will be used frequently in relation to 
>> the #plugins x #classes.
> 
> Ah, i see, so the rule being: go back to the pool to query for stuff.
> 
> How about the following helper method on ModulePool:
> 
>   Optional findModule(ModuleEntry me);
> 
> then the intent in code might be a littler clearer on the context.
> 

Ok

> 
> ImagePluginStack
> 
> 407 return res.isPresent()? 
> Optional.of(getUncompressed(res.get())) : Optional.empty();
> 
> You could do:
> 
>  return res.map(this::getUncompressed);
> 

Good


> 
> ClassForNamePLugin
> 
> 148 .filter(resource -> resource != null)
> 
> Can a resource ever be null?

I don't think it can. Just cloning JFs code. Might have true at some point. 
Will drop and see what falls out. 

> 
> Paul.

Re: JDK 9 RFR to remove jdk/internal/jimage/JImageReadTest.java from the problem list

[James Laskey]

I meant proceed but note there is a concern about why these resources are 
present now and not prior to the sjavac changes. 

Sent from my iPhone

> On Jan 9, 2016, at 4:55 PM, joe darcy <joe.da...@oracle.com> wrote:
> 
> Hi Jim,
> 
> There may or may not be a lurking issue, but as it stand the test 
> JImageReadTest.java is not being run in any of the official test runs since 
> it is currently on the exclude list.
> 
> -Joe
> 
>> On 1/9/2016 11:35 AM, James Laskey wrote:
>> Alan questioned why this is showing up after the sjavac changes. There was 
>> concern there was some lurking issue.
>> 
>> Sent from my iPhone
>> 
>>> On Jan 9, 2016, at 3:19 PM, joe darcy <joe.da...@oracle.com> wrote:
>>> 
>>> Hello,
>>> 
>>> The recent push of the fix for
>>> 
>>>8146712: jdk/internal/jimage/JImageReadTest.java fails on all platforms
>>> 
>>> did not remove JImageReadTest.java from the problem list.
>>> 
>>> Please review the patch below to restore the test to the set of tests that 
>>> get run.
>>> 
>>> Thanks,
>>> 
>>> -Joe
>>> 
>>> --- a/test/ProblemList.txtSat Jan 09 11:19:32 2016 -0400
>>> +++ b/test/ProblemList.txtSat Jan 09 11:14:50 2016 -0800
>>> @@ -384,13 +384,6 @@
>>> 
>>> 
>>> 
>>> -# core_tools
>>> -
>>> -# 8146712
>>> -jdk/internal/jimage/JImageReadTest.java  generic-all
>>> -
>>> -
>>> -
>>> # svc_tools
>>> 
>>> # 8031482
> 

Re: JDK 9 RFR to remove jdk/internal/jimage/JImageReadTest.java from the problem list

[James Laskey]

Alan questioned why this is showing up after the sjavac changes. There was 
concern there was some lurking issue. 

Sent from my iPhone

> On Jan 9, 2016, at 3:19 PM, joe darcy  wrote:
> 
> Hello,
> 
> The recent push of the fix for
> 
>8146712: jdk/internal/jimage/JImageReadTest.java fails on all platforms
> 
> did not remove JImageReadTest.java from the problem list.
> 
> Please review the patch below to restore the test to the set of tests that 
> get run.
> 
> Thanks,
> 
> -Joe
> 
> --- a/test/ProblemList.txtSat Jan 09 11:19:32 2016 -0400
> +++ b/test/ProblemList.txtSat Jan 09 11:14:50 2016 -0800
> @@ -384,13 +384,6 @@
> 
> 
> 
> -# core_tools
> -
> -# 8146712
> -jdk/internal/jimage/JImageReadTest.java  generic-all
> -
> -
> -
> # svc_tools
> 
> # 8031482
> 

hg: jdk8/tl/nashorn: 3 new changesets

[james . laskey]

Changeset: 7985ec3782b5
Author:hannesw
Date:  2013-10-25 10:20 +0200
URL:   http://hg.openjdk.java.net/jdk8/tl/nashorn/rev/7985ec3782b5

8027042: Evaluation order for binary operators can be improved
Reviewed-by: lagergren, jlaskey, attila

! src/jdk/nashorn/internal/codegen/CodeGenerator.java
! src/jdk/nashorn/internal/codegen/types/Type.java
! src/jdk/nashorn/internal/ir/BinaryNode.java
! src/jdk/nashorn/internal/ir/Expression.java
! src/jdk/nashorn/internal/ir/IdentNode.java
! src/jdk/nashorn/internal/ir/LiteralNode.java
! src/jdk/nashorn/internal/ir/TernaryNode.java
! src/jdk/nashorn/internal/ir/UnaryNode.java
+ test/script/basic/JDK-8027042.js
+ test/script/basic/JDK-8027042.js.EXPECTED

Changeset: 71cfb21c68dc
Author:hannesw
Date:  2013-10-25 15:21 +0200
URL:   http://hg.openjdk.java.net/jdk8/tl/nashorn/rev/71cfb21c68dc

8027301: Optimizations for Function.prototype.apply
Reviewed-by: jlaskey

! src/jdk/nashorn/internal/runtime/CompiledFunctions.java
! src/jdk/nashorn/internal/runtime/FinalScriptFunctionData.java
! src/jdk/nashorn/internal/runtime/ScriptFunctionData.java

Changeset: 406f2b672937
Author:jlaskey
Date:  2013-10-29 10:40 -0300
URL:   http://hg.openjdk.java.net/jdk8/tl/nashorn/rev/406f2b672937

Merge

hg: jdk8/tl: 26 new changesets

[james . laskey]

Changeset: 3dc55f0c1b6f
Author:jlaskey
Date:  2013-01-28 16:29 -0400
URL:   http://hg.openjdk.java.net/jdk8/tl/rev/3dc55f0c1b6f

8006676: Integrate Nashorn into new build system
Reviewed-by: jlaskey
Contributed-by: james.las...@oracle.com

! common/autoconf/generated-configure.sh
! common/autoconf/source-dirs.m4
! common/autoconf/spec.gmk.in
! common/bin/compare.sh
! common/makefiles/Main.gmk
! common/makefiles/MakeBase.gmk
+ make/nashorn-rules.gmk
! make/scripts/hgforest.sh

Changeset: ecd447139a39
Author:jlaskey
Date:  2013-02-04 17:30 -0400
URL:   http://hg.openjdk.java.net/jdk8/tl/rev/ecd447139a39

Merge

! common/autoconf/generated-configure.sh

Changeset: 9ed388a04fa7
Author:jlaskey
Date:  2013-02-06 13:37 -0400
URL:   http://hg.openjdk.java.net/jdk8/tl/rev/9ed388a04fa7

8007666: nashorn missing from hgforest.sh
Reviewed-by: jlaskey
Contributed-by: james.las...@oracle.com

! common/bin/hgforest.sh

Changeset: 8b19b55f695d
Author:jlaskey
Date:  2013-02-18 19:01 -0400
URL:   http://hg.openjdk.java.net/jdk8/tl/rev/8b19b55f695d

8008420: Fix Nashorn forest to build with NEWBUILD=false
Reviewed-by: jjh
Contributed-by: james.las...@oracle.com

! Makefile
! make/nashorn-rules.gmk

Changeset: f9a1cb245484
Author:jlaskey
Date:  2013-02-19 10:02 -0400
URL:   http://hg.openjdk.java.net/jdk8/tl/rev/f9a1cb245484

8008446: Tweaks to make all NEWBUILD=false round 2
Reviewed-by: jjh
Contributed-by: james.las...@oracle.com

! make/Defs-internal.gmk

Changeset: 887fde71977e
Author:jlaskey
Date:  2013-02-21 15:25 -0400
URL:   http://hg.openjdk.java.net/jdk8/tl/rev/887fde71977e

8008447: Tweaks to make all NEWBUILD=false round 3
Reviewed-by: jjh, sundar
Contributed-by: james.las...@oracle.com

! make/jdk-rules.gmk
! make/sanity-rules.gmk

Changeset: e877cb3eb4d6
Author:jlaskey
Date:  2013-02-22 13:09 -0400
URL:   http://hg.openjdk.java.net/jdk8/tl/rev/e877cb3eb4d6

Merge

! common/autoconf/generated-configure.sh
! common/autoconf/spec.gmk.in
! common/bin/hgforest.sh
! common/makefiles/Main.gmk

Changeset: 528a9984198f
Author:jlaskey
Date:  2013-02-22 22:58 -0400
URL:   http://hg.openjdk.java.net/jdk8/tl/rev/528a9984198f

8008774: nashorn missing from dependencies after merge with tl
Reviewed-by: jjh
Contributed-by: james.las...@oracle.com

! common/makefiles/Main.gmk

Changeset: 13ddc5c3ebfc
Author:jlaskey
Date:  2013-03-02 10:28 -0400
URL:   http://hg.openjdk.java.net/jdk8/tl/rev/13ddc5c3ebfc

Merge

! common/autoconf/generated-configure.sh
! common/autoconf/spec.gmk.in
! common/bin/hgforest.sh
! common/makefiles/Main.gmk
! make/nashorn-rules.gmk

Changeset: 1d38d30196be
Author:jlaskey
Date:  2013-03-08 14:44 -0400
URL:   http://hg.openjdk.java.net/jdk8/tl/rev/1d38d30196be

Merge

! common/autoconf/generated-configure.sh
! common/autoconf/spec.gmk.in
! common/makefiles/Main.gmk

Changeset: b938d5ee29c3
Author:jlaskey
Date:  2013-03-15 11:50 -0300
URL:   http://hg.openjdk.java.net/jdk8/tl/rev/b938d5ee29c3

Merge


Changeset: fe5a388bf8fe
Author:jlaskey
Date:  2013-03-26 09:13 -0300
URL:   http://hg.openjdk.java.net/jdk8/tl/rev/fe5a388bf8fe

Merge

! common/autoconf/generated-configure.sh
! common/autoconf/spec.gmk.in
! common/makefiles/Main.gmk

Changeset: 1378ccca1c79
Author:jlaskey
Date:  2013-04-09 08:35 -0300
URL:   http://hg.openjdk.java.net/jdk8/tl/rev/1378ccca1c79

Merge

! common/autoconf/generated-configure.sh
! common/autoconf/spec.gmk.in
! common/bin/hgforest.sh
! common/makefiles/Main.gmk

Changeset: 8a7e97848471
Author:jlaskey
Date:  2013-04-15 08:06 -0300
URL:   http://hg.openjdk.java.net/jdk8/tl/rev/8a7e97848471

Merge

! common/autoconf/generated-configure.sh
! common/autoconf/spec.gmk.in
! common/makefiles/Main.gmk

Changeset: 6316aefcf716
Author:jlaskey
Date:  2013-04-17 08:47 -0300
URL:   http://hg.openjdk.java.net/jdk8/tl/rev/6316aefcf716

Merge


Changeset: dd81e9b5fb38
Author:jlaskey
Date:  2013-04-22 13:59 -0300
URL:   http://hg.openjdk.java.net/jdk8/tl/rev/dd81e9b5fb38

Merge


Changeset: 431d16ddfcd9
Author:jlaskey
Date:  2013-04-29 21:37 -0300
URL:   http://hg.openjdk.java.net/jdk8/tl/rev/431d16ddfcd9

Merge


Changeset: 1fca390200c1
Author:jlaskey
Date:  2013-05-14 09:04 -0300
URL:   http://hg.openjdk.java.net/jdk8/tl/rev/1fca390200c1

Merge

! common/autoconf/generated-configure.sh
! common/autoconf/spec.gmk.in
! common/makefiles/Main.gmk

Changeset: 67b5cbe55744
Author:jlaskey
Date:  2013-05-23 09:48 -0300
URL:   http://hg.openjdk.java.net/jdk8/tl/rev/67b5cbe55744

Merge


Changeset: de886178f8e6
Author:jlaskey
Date:  2013-06-05 13:08 -0300
URL:   http://hg.openjdk.java.net/jdk8/tl/rev/de886178f8e6

Merge

! common/autoconf/generated-configure.sh
! common/autoconf/spec.gmk.in

Changeset: 520fd54a7c43
Author:jlaskey
Date:  2013-07-16 09:08 -0300

hg: jdk8/tl/jdk: 27 new changesets

[james . laskey]

Changeset: a8bbd962f34a
Author:jlaskey
Date:  2013-01-28 16:29 -0400
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/a8bbd962f34a

8006676: Integrate Nashorn into new build system
Reviewed-by: jlaskey
Contributed-by: james.las...@oracle.com

! THIRD_PARTY_README
! make/launchers/Makefile
! makefiles/CompileLaunchers.gmk
! makefiles/CreateJars.gmk
! test/tools/launcher/VersionCheck.java

Changeset: 41654275896d
Author:jlaskey
Date:  2013-02-04 17:29 -0400
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/41654275896d

Merge

! makefiles/CompileLaunchers.gmk
! makefiles/CreateJars.gmk
- src/share/classes/java/lang/annotation/ContainedBy.java
- src/share/classes/java/lang/annotation/ContainerFor.java
- test/java/net/URL/abnormal_http_urls
- test/java/net/URL/ftp_urls
- test/java/net/URL/jar_urls
- test/java/net/URL/normal_http_urls
- test/java/net/URL/runconstructor.sh
- test/java/net/URL/share_file_urls
- test/java/net/URL/win32_file_urls
- test/sun/net/www/EncDec.doc
- test/sun/net/www/MarkResetTest.java
- test/sun/net/www/MarkResetTest.sh
- test/sun/security/util/Oid/S11N.sh
- test/sun/security/util/Oid/SerialTest.java
! test/tools/launcher/VersionCheck.java

Changeset: a174944b0c00
Author:jlaskey
Date:  2013-02-21 15:25 -0400
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/a174944b0c00

8008447: Tweaks to make all NEWBUILD=false round 3
Reviewed-by: jjh, sundar
Contributed-by: james.las...@oracle.com

! make/launchers/Makefile

Changeset: 25db7658a063
Author:jlaskey
Date:  2013-02-22 10:23 -0400
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/25db7658a063

8008721: Tweaks to make all NEWBUILD=false round 4
Reviewed-by: jjh
Contributed-by: james.las...@oracle.com

! make/launchers/Makefile

Changeset: ea22045ce09b
Author:jlaskey
Date:  2013-02-22 14:05 -0400
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/ea22045ce09b

Merge

! makefiles/CreateJars.gmk

Changeset: ff68fafd6302
Author:jlaskey
Date:  2013-02-22 17:45 -0400
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/ff68fafd6302

8008756: THIRD_PARTY_README contains Unicode
Reviewed-by: jjh
Contributed-by: james.las...@oracle.com

! THIRD_PARTY_README

Changeset: ce82a0ee7735
Author:jlaskey
Date:  2013-02-22 18:03 -0400
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/ce82a0ee7735

8008757: NEWBUILD=true has separate launcher code for jjs
Reviewed-by: jjh
Contributed-by: james.las...@oracle.com

! makefiles/CompileLaunchers.gmk

Changeset: 20a827b22a2e
Author:jlaskey
Date:  2013-02-22 23:36 -0400
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/20a827b22a2e

8008775: Remove non-ascii from jdk/THIRD_PARTY_README
Reviewed-by: jjh
Contributed-by: james.las...@oracle.com

! THIRD_PARTY_README

Changeset: 364e0871f7a3
Author:jlaskey
Date:  2013-03-02 11:06 -0400
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/364e0871f7a3

Merge

! make/launchers/Makefile
! makefiles/CompileLaunchers.gmk
! makefiles/CreateJars.gmk
- src/share/classes/java/lang/annotation/InvalidContainerAnnotationError.java
- test/javax/script/RhinoExceptionTest.java
! test/tools/launcher/VersionCheck.java

Changeset: 3565c755c49f
Author:jlaskey
Date:  2013-03-15 11:51 -0300
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/3565c755c49f

Merge

! makefiles/CreateJars.gmk

Changeset: 8c223a4f906a
Author:jlaskey
Date:  2013-03-26 09:12 -0300
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/8c223a4f906a

Merge

! makefiles/CreateJars.gmk

Changeset: efbbcd5848cf
Author:jlaskey
Date:  2013-04-01 10:09 -0300
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/efbbcd5848cf

Merge


Changeset: 39ce82dad57d
Author:jlaskey
Date:  2013-04-09 08:36 -0300
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/39ce82dad57d

Merge


Changeset: ff9683b6854c
Author:jlaskey
Date:  2013-04-15 08:27 -0300
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/ff9683b6854c

Merge

! makefiles/CompileLaunchers.gmk
! makefiles/CreateJars.gmk

Changeset: 1cdf20da340b
Author:jlaskey
Date:  2013-04-17 08:47 -0300
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/1cdf20da340b

Merge


Changeset: 72fa581a83a4
Author:jlaskey
Date:  2013-04-22 14:00 -0300
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/72fa581a83a4

Merge


Changeset: ead9944bbb3b
Author:jlaskey
Date:  2013-04-29 21:37 -0300
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/ead9944bbb3b

Merge


Changeset: 5bde43b1e463
Author:jlaskey
Date:  2013-05-14 09:04 -0300
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/5bde43b1e463

Merge


Changeset: efd620f8963f
Author:jlaskey
Date:  2013-05-23 09:48 -0300
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/efd620f8963f

Merge

! makefiles/CreateJars.gmk

Changeset: 193652dff077
Author:jlaskey
Date:  2013-05-29 13:22 -0300
URL:   

hg: jdk8/tl/nashorn: 6 new changesets

[james . laskey]

Changeset: 0feca8a93cb3
Author:attila
Date:  2013-06-05 10:44 +0200
URL:   http://hg.openjdk.java.net/jdk8/tl/nashorn/rev/0feca8a93cb3

8015955: ObjectNode.elements should be stronger typed
Reviewed-by: lagergren, sundar

! src/jdk/nashorn/internal/codegen/CodeGenerator.java
! src/jdk/nashorn/internal/ir/BlockLexicalContext.java
! src/jdk/nashorn/internal/ir/ObjectNode.java
! src/jdk/nashorn/internal/parser/JSONParser.java
! src/jdk/nashorn/internal/parser/Parser.java
! src/jdk/nashorn/internal/runtime/JSONFunctions.java

Changeset: 9374c04f38fe
Author:attila
Date:  2013-06-05 12:17 +0200
URL:   http://hg.openjdk.java.net/jdk8/tl/nashorn/rev/9374c04f38fe

8015961: Several small code-gardening fixes
Reviewed-by: lagergren, sundar

! src/jdk/nashorn/internal/codegen/Lower.java
! src/jdk/nashorn/internal/codegen/RuntimeCallSite.java
! src/jdk/nashorn/internal/ir/FunctionNode.java
! src/jdk/nashorn/internal/objects/GenericPropertyDescriptor.java
! src/jdk/nashorn/internal/objects/NativeMath.java
! src/jdk/nashorn/internal/runtime/Context.java
! src/jdk/nashorn/internal/runtime/ListAdapter.java
! src/jdk/nashorn/internal/runtime/regexp/joni/Parser.java

Changeset: 60bc560df392
Author:hannesw
Date:  2013-06-05 12:44 +0200
URL:   http://hg.openjdk.java.net/jdk8/tl/nashorn/rev/60bc560df392

8015350: Array.prototype.reduceRight issue with large length and index
Reviewed-by: attila, sundar, lagergren

! src/jdk/nashorn/internal/objects/NativeArray.java
! src/jdk/nashorn/internal/runtime/arrays/ArrayIterator.java
! src/jdk/nashorn/internal/runtime/arrays/ArrayLikeIterator.java
! src/jdk/nashorn/internal/runtime/arrays/EmptyArrayLikeIterator.java
! src/jdk/nashorn/internal/runtime/arrays/IteratorAction.java
! src/jdk/nashorn/internal/runtime/arrays/MapIterator.java
! src/jdk/nashorn/internal/runtime/arrays/ReverseArrayIterator.java
! src/jdk/nashorn/internal/runtime/arrays/ReverseMapIterator.java
+ test/script/basic/JDK-8015350.js
+ test/script/basic/JDK-8015350.js.EXPECTED

Changeset: 35bba63990b7
Author:jlaskey
Date:  2013-06-05 10:32 -0300
URL:   http://hg.openjdk.java.net/jdk8/tl/nashorn/rev/35bba63990b7

8015911: $EXEC does not handle large outputs
Reviewed-by: sundar, attila
Contributed-by: james.las...@oracle.com

! src/jdk/nashorn/internal/runtime/ScriptingFunctions.java

Changeset: 16219bef66ec
Author:jlaskey
Date:  2013-06-05 12:41 -0300
URL:   http://hg.openjdk.java.net/jdk8/tl/nashorn/rev/16219bef66ec

8015910: Nashorn JavaFX includes are out of sync with JavaFX repo
Reviewed-by: sundar
Contributed-by: james.las...@oracle.com

! src/jdk/nashorn/internal/runtime/resources/fx/controls.js
! src/jdk/nashorn/internal/runtime/resources/fx/graphics.js
! src/jdk/nashorn/internal/runtime/resources/fx/swt.js
! src/jdk/nashorn/internal/runtime/resources/fx/web.js

Changeset: e3bd0ed64da8
Author:jlaskey
Date:  2013-06-05 12:54 -0300
URL:   http://hg.openjdk.java.net/jdk8/tl/nashorn/rev/e3bd0ed64da8

Merge

hg: jdk8/tl/nashorn: 9 new changesets

[james . laskey]

Changeset: 0bf451c0678d
Author:hannesw
Date:  2013-05-27 12:26 +0200
URL:   http://hg.openjdk.java.net/jdk8/tl/nashorn/rev/0bf451c0678d

8015348: RegExp([) results in StackOverflowError
Reviewed-by: sundar, attila

! src/jdk/nashorn/internal/runtime/regexp/RegExpScanner.java
+ test/script/basic/JDK-8015348.js
+ test/script/basic/JDK-8015348.js.EXPECTED

Changeset: 1f57afd14cc1
Author:lagergren
Date:  2013-05-27 13:11 +0200
URL:   http://hg.openjdk.java.net/jdk8/tl/nashorn/rev/1f57afd14cc1

8014219: Make the run-octane harness more deterministic by not measuring 
elapsed time every iteration. Also got rid of most of the run logic in base.js 
and call benchmarks directly for the same purpose
Reviewed-by: jlaskey, attila

! make/build-benchmark.xml
! src/jdk/nashorn/internal/runtime/AccessorProperty.java
! src/jdk/nashorn/internal/runtime/Property.java
! src/jdk/nashorn/internal/runtime/UserAccessorProperty.java
! test/script/basic/compile-octane.js.EXPECTED
! test/script/basic/run-octane.js

Changeset: 910fd2849c4c
Author:lagergren
Date:  2013-05-27 13:12 +0200
URL:   http://hg.openjdk.java.net/jdk8/tl/nashorn/rev/910fd2849c4c

Merge


Changeset: 343fd0450802
Author:sundar
Date:  2013-05-27 20:41 +0530
URL:   http://hg.openjdk.java.net/jdk8/tl/nashorn/rev/343fd0450802

8015352: i.toUpperCase() = currently returns Ä°, but should be I (with 
Turkish locale)
Reviewed-by: jlaskey, lagergren

! src/jdk/nashorn/internal/objects/NativeString.java
! src/jdk/nashorn/internal/runtime/ScriptEnvironment.java
! src/jdk/nashorn/internal/runtime/options/OptionTemplate.java
! src/jdk/nashorn/internal/runtime/options/Options.java
! src/jdk/nashorn/internal/runtime/resources/Options.properties
+ test/script/basic/JDK-8015352.js

Changeset: e6193dcfe36c
Author:lagergren
Date:  2013-05-27 17:57 +0200
URL:   http://hg.openjdk.java.net/jdk8/tl/nashorn/rev/e6193dcfe36c

8015447: Octane harness fixes for rhino and entire test runs: ant octane, ant 
octane-v8, ant octane-rhino
Reviewed-by: sundar, jlaskey

! make/build-benchmark.xml
! test/script/basic/run-octane.js

Changeset: d56168970de1
Author:sundar
Date:  2013-05-28 16:37 +0530
URL:   http://hg.openjdk.java.net/jdk8/tl/nashorn/rev/d56168970de1

8015459: Octane test run fails on Turkish locale
Reviewed-by: lagergren, attila

! src/jdk/nashorn/internal/codegen/CodeGenerator.java
! src/jdk/nashorn/internal/objects/DateParser.java
! src/jdk/nashorn/internal/parser/TokenType.java
! src/jdk/nashorn/internal/runtime/GlobalFunctions.java
! src/jdk/nashorn/internal/runtime/JSType.java
! src/jdk/nashorn/internal/runtime/Logging.java
! src/jdk/nashorn/internal/runtime/ScriptRuntime.java
! src/jdk/nashorn/internal/runtime/options/OptionTemplate.java

Changeset: f472f7046ec9
Author:sundar
Date:  2013-05-29 15:41 +0530
URL:   http://hg.openjdk.java.net/jdk8/tl/nashorn/rev/f472f7046ec9

8005979: A lot of tests are named runTest in reports
Reviewed-by: jlaskey

! make/project.properties

Changeset: f69e76417211
Author:lagergren
Date:  2013-05-29 14:08 +0200
URL:   http://hg.openjdk.java.net/jdk8/tl/nashorn/rev/f69e76417211

8011023: Math round didn't conform to ECMAScript 5 spec
Reviewed-by: jlaskey, attila

! src/jdk/nashorn/internal/objects/NativeMath.java
+ test/script/basic/JDK-8011023.js
+ test/script/basic/JDK-8011023.js.EXPECTED

Changeset: a2e2797392b3
Author:sundar
Date:  2013-05-29 21:27 +0530
URL:   http://hg.openjdk.java.net/jdk8/tl/nashorn/rev/a2e2797392b3

8015349: abc.lastIndexOf(a,-1) should evaluate to 0 and not -1
Reviewed-by: lagergren, attila, jlaskey

! src/jdk/nashorn/internal/objects/NativeString.java
+ test/script/basic/JDK-8015349.js
+ test/script/basic/JDK-8015349.js.EXPECTED

hg: jdk8/tl/nashorn: 9 new changesets

[james . laskey]

Changeset: 833a9a584b64
Author:attila
Date:  2013-05-21 13:40 +0200
URL:   http://hg.openjdk.java.net/jdk8/tl/nashorn/rev/833a9a584b64

8014953: Have NativeJavaPackage throw a ClassNotFoundException when invoked
Reviewed-by: jlaskey, sundar

! src/jdk/nashorn/internal/runtime/NativeJavaPackage.java
+ test/script/basic/JDK-8014953.js
+ test/script/basic/JDK-8014953.js.EXPECTED

Changeset: 288ff54da2a5
Author:jlaskey
Date:  2013-05-21 10:17 -0300
URL:   http://hg.openjdk.java.net/jdk8/tl/nashorn/rev/288ff54da2a5

8014827: readLine should accept a prompt as an argument
Reviewed-by: sundar, hannesw
Contributed-by: james.las...@oracle.com

! src/jdk/nashorn/internal/runtime/ScriptingFunctions.java

Changeset: 07cefc062032
Author:sundar
Date:  2013-05-22 16:39 +0530
URL:   http://hg.openjdk.java.net/jdk8/tl/nashorn/rev/07cefc062032

8008947: ScriptEnvironment ctor should be public
Reviewed-by: lagergren, attila

! .hgignore
! src/jdk/nashorn/internal/runtime/ScriptEnvironment.java

Changeset: 66685c69bdb3
Author:sundar
Date:  2013-05-22 19:33 +0530
URL:   http://hg.openjdk.java.net/jdk8/tl/nashorn/rev/66685c69bdb3

8014735: Typed Array, BYTES_PER_ELEMENT should be a class property
Reviewed-by: lagergren, jlaskey

! src/jdk/nashorn/internal/objects/ArrayBufferView.java
! src/jdk/nashorn/internal/objects/NativeFloat32Array.java
! src/jdk/nashorn/internal/objects/NativeFloat64Array.java
! src/jdk/nashorn/internal/objects/NativeInt16Array.java
! src/jdk/nashorn/internal/objects/NativeInt32Array.java
! src/jdk/nashorn/internal/objects/NativeInt8Array.java
! src/jdk/nashorn/internal/objects/NativeUint16Array.java
! src/jdk/nashorn/internal/objects/NativeUint32Array.java
! src/jdk/nashorn/internal/objects/NativeUint8Array.java
! src/jdk/nashorn/internal/objects/NativeUint8ClampedArray.java
+ test/script/basic/JDK-8014735.js
+ test/script/basic/JDK-8014735.js.EXPECTED
! test/script/basic/NASHORN-377.js

Changeset: 8f7553df4503
Author:hannesw
Date:  2013-05-22 16:43 +0200
URL:   http://hg.openjdk.java.net/jdk8/tl/nashorn/rev/8f7553df4503

8010804: Review long and integer usage conventions
Reviewed-by: jlaskey, sundar

! src/jdk/nashorn/internal/codegen/FoldConstants.java
! src/jdk/nashorn/internal/objects/NativeArray.java
! src/jdk/nashorn/internal/objects/NativeDate.java
! src/jdk/nashorn/internal/runtime/JSType.java
+ test/script/basic/JDK-8010804.js
+ test/script/basic/JDK-8010804.js.EXPECTED

Changeset: 1c1453863ea8
Author:attila
Date:  2013-05-23 12:01 +0200
URL:   http://hg.openjdk.java.net/jdk8/tl/nashorn/rev/1c1453863ea8

8015267: Allow conversion of JS arrays to Java List/Deque
Reviewed-by: lagergren, sundar

! make/build.xml
! src/jdk/nashorn/internal/objects/NativeJava.java
+ src/jdk/nashorn/internal/runtime/ListAdapter.java
! src/jdk/nashorn/internal/runtime/linker/InvokeByName.java
! src/jdk/nashorn/internal/runtime/resources/Messages.properties
+ test/script/basic/JDK-8015267.js
+ test/script/basic/JDK-8015267.js.EXPECTED

Changeset: f7eb4436410e
Author:lagergren
Date:  2013-05-23 13:10 +0200
URL:   http://hg.openjdk.java.net/jdk8/tl/nashorn/rev/f7eb4436410e

8012083: Array literal constant folding issue
Reviewed-by: attila, jlaskey

! src/jdk/nashorn/internal/codegen/CodeGenerator.java
! src/jdk/nashorn/internal/codegen/FinalizeTypes.java
! src/jdk/nashorn/internal/codegen/FoldConstants.java
+ test/script/basic/JDK-8012083.js
+ test/script/basic/JDK-8012083.js.EXPECTED

Changeset: 704bc91a0c41
Author:attila
Date:  2013-05-23 13:36 +0200
URL:   http://hg.openjdk.java.net/jdk8/tl/nashorn/rev/704bc91a0c41

8015278: Revert accidental changes to build.xml
Reviewed-by: jlaskey, lagergren

! make/build.xml

Changeset: 8af550dee961
Author:jlaskey
Date:  2013-05-23 09:49 -0300
URL:   http://hg.openjdk.java.net/jdk8/tl/nashorn/rev/8af550dee961

Merge

hg: jdk8/tl/nashorn: 17 new changesets

[james . laskey]

Changeset: b754fb89367d
Author:jlaskey
Date:  2013-04-30 10:05 -0300
URL:   http://hg.openjdk.java.net/jdk8/tl/nashorn/rev/b754fb89367d

8006220: Simplify PropertyMaps
Reviewed-by: hannesw, lagergren
Contributed-by: james.las...@oracle.com

! src/jdk/nashorn/internal/codegen/MapCreator.java
! src/jdk/nashorn/internal/codegen/ObjectClassGenerator.java
! src/jdk/nashorn/internal/codegen/ObjectCreator.java
! src/jdk/nashorn/internal/objects/NativeDebug.java
! src/jdk/nashorn/internal/objects/NativeJSAdapter.java
! src/jdk/nashorn/internal/runtime/AccessorProperty.java
! src/jdk/nashorn/internal/runtime/Context.java
! src/jdk/nashorn/internal/runtime/Property.java
! src/jdk/nashorn/internal/runtime/PropertyHashMap.java
! src/jdk/nashorn/internal/runtime/PropertyMap.java
! src/jdk/nashorn/internal/runtime/ScriptObject.java
! src/jdk/nashorn/internal/runtime/SetMethodCreator.java
- src/jdk/nashorn/internal/runtime/SpillProperty.java
! src/jdk/nashorn/internal/runtime/StructureLoader.java
! src/jdk/nashorn/internal/runtime/UserAccessorProperty.java
! src/jdk/nashorn/internal/scripts/JO.java
! src/jdk/nashorn/tools/Shell.java

Changeset: 80cb02dedc83
Author:hannesw
Date:  2013-05-02 09:19 +0200
URL:   http://hg.openjdk.java.net/jdk8/tl/nashorn/rev/80cb02dedc83

8013729: SwitchPoint invalidation not working over prototype chain
Reviewed-by: lagergren, sundar

! src/jdk/nashorn/internal/runtime/ScriptObject.java
+ test/script/basic/JDK-8013729.js
+ test/script/basic/JDK-8013729.js.EXPECTED

Changeset: 7563c56ca565
Author:jlaskey
Date:  2013-05-02 13:22 -0300
URL:   http://hg.openjdk.java.net/jdk8/tl/nashorn/rev/7563c56ca565

8013794: JDK-8006220 caused an octane performance regression.
Reviewed-by: lagergren, sundar
Contributed-by: james.las...@oracle.com

! src/jdk/nashorn/internal/codegen/ObjectCreator.java

Changeset: 9c2376a250b6
Author:jlaskey
Date:  2013-05-02 13:23 -0300
URL:   http://hg.openjdk.java.net/jdk8/tl/nashorn/rev/9c2376a250b6

Merge


Changeset: c8023561505b
Author:jlaskey
Date:  2013-05-02 15:01 -0300
URL:   http://hg.openjdk.java.net/jdk8/tl/nashorn/rev/c8023561505b

8013796: load(fx:base.js) should not be in fx:bootstrap.js
Reviewed-by: sundar, lagergren
Contributed-by: james.las...@oracle.com

! src/jdk/nashorn/internal/runtime/resources/fx/bootstrap.js

Changeset: 5a3f7867e19c
Author:lagergren
Date:  2013-05-03 15:33 +0200
URL:   http://hg.openjdk.java.net/jdk8/tl/nashorn/rev/5a3f7867e19c

8013477: Node.setSymbol needs to be copy on write - enable IR snapshots for 
recompilation based on callsite type specialization. [not enabled by default, 
hidden by a flag for now]
Reviewed-by: jlaskey, hannesw

! bin/jjs
! src/jdk/nashorn/api/scripting/NashornScriptEngineFactory.java
! src/jdk/nashorn/internal/codegen/Attr.java
! src/jdk/nashorn/internal/codegen/CodeGenerator.java
! src/jdk/nashorn/internal/codegen/CompilationPhase.java
! src/jdk/nashorn/internal/codegen/Compiler.java
! src/jdk/nashorn/internal/codegen/FinalizeTypes.java
! src/jdk/nashorn/internal/codegen/ObjectCreator.java
! src/jdk/nashorn/internal/codegen/Splitter.java
! src/jdk/nashorn/internal/ir/Block.java
! src/jdk/nashorn/internal/ir/CatchNode.java
! src/jdk/nashorn/internal/ir/FunctionNode.java
! src/jdk/nashorn/internal/ir/LexicalContext.java
! src/jdk/nashorn/internal/ir/LexicalContextNode.java
! src/jdk/nashorn/internal/ir/LiteralNode.java
! src/jdk/nashorn/internal/ir/Node.java
! src/jdk/nashorn/internal/ir/Symbol.java
! src/jdk/nashorn/internal/objects/NativeRegExp.java
! src/jdk/nashorn/internal/parser/AbstractParser.java
! src/jdk/nashorn/internal/parser/Parser.java
! src/jdk/nashorn/internal/runtime/CompiledFunction.java
! src/jdk/nashorn/internal/runtime/CompiledFunctions.java
! src/jdk/nashorn/internal/runtime/Context.java
! src/jdk/nashorn/internal/runtime/JSONFunctions.java
! src/jdk/nashorn/internal/runtime/RecompilableScriptFunctionData.java
! src/jdk/nashorn/internal/runtime/ScriptEnvironment.java
! src/jdk/nashorn/internal/runtime/ScriptObject.java
! src/jdk/nashorn/internal/runtime/regexp/DefaultRegExp.java
! src/jdk/nashorn/internal/runtime/regexp/JoniRegExp.java
! src/jdk/nashorn/internal/runtime/regexp/RegExpScanner.java
! src/jdk/nashorn/internal/runtime/resources/Options.properties
! src/jdk/nashorn/tools/Shell.java
+ test/script/basic/paramspec.js
+ test/script/basic/paramspec.js.EXPECTED
! test/script/basic/runsunspider.js
+ test/script/currently-failing/logcoverage.js
- test/script/trusted/logcoverage.js

Changeset: 829b06307fb2
Author:lagergren
Date:  2013-05-03 16:01 +0200
URL:   http://hg.openjdk.java.net/jdk8/tl/nashorn/rev/829b06307fb2

8013871: mem usage histograms enabled with compiler logging level set to more 
specific than or equals to info when --print-mem-usage flag is used
Reviewed-by: jlaskey, hannesw

! src/jdk/nashorn/internal/codegen/Compiler.java
+