Re: RFR: JDK-8035870 - Check jdk/src/windows/native/java/io/WinNTFileSystem_md.c for JNI pending exceptions
Looks good to me Mark. -Chris. On 12 Mar 2014, at 17:39, Mark Sheppard mark.shepp...@oracle.com wrote: Hi please oblige and review the following changes http://cr.openjdk.java.net/~msheppar/8035870/webrev/ which address the issues raised in https://bugs.openjdk.java.net/browse/JDK-8035870 summary: checks to return values from malloc added with appropriate JNU_ThrowOutOfMemoryError added regards Mark
Re: RFR: JDK9: 8037221: [asm] refresh internal ASM version
Hi, Eye-balled the changes, nothing obvious popped out. Looks good. It's very minor, and should not block the code going back, but occured is a common misspelling of occurred. Paul. On Mar 12, 2014, at 10:24 PM, Kumar Srinivasan kumar.x.sriniva...@oracle.com wrote: Hello, Appreciate if someone cane review this change, so that I can push this into the jdk9 repository, refreshing the internal ASM sources from rev 1700 to 1721, of particular interest is Rev 1710, which fixes [1] known to cause verification errors in the VM. Thereafter I will be back-porting this changeset into 8u20. http://cr.openjdk.java.net/~ksrini/8037221/ The following regression tests have been run: nashorn/test jdk/test/tools/pack200 jdk/test/java/lang/invoke jdk/test/java/util jdk/test/javax/script jdk/test/sun/tools/jrunscript jdk/test/closed/com/oracle/jfr langtools/test Nashorn additional test targets: test test262 octane Thanks in advance Kumar [1] http://forge.ow2.org/tracker/?group_id=23atid=100023func=detailaid=316545
Re: Unsafe: removing the monitorEnter/monitorExit/tryMonitorEnter methods
On Mar 12, 2014, at 10:23 PM, John Rose john.r.r...@oracle.com wrote: P.S. FTR, I wish we could also remove the per-object monitor from its privileged position, so not all objects are burdened by it, and other forms of lock can be switched into the existing notation on a type-by-type basis. This is obviously a long, long term wish. A starting point is a notion of this object has no monitor. Like an instance of a value type? Paul. Probably the keyword cannot be repurposed but would eventually be deprecated. And the Moon will reach Roche limit. – John On Mar 12, 2014, at 2:12 PM, John Rose john.r.r...@oracle.com wrote: I think that we've covered all the angles, and that we can remove it. – John On Mar 12, 2014, at 1:44 PM, Paul Sandoz paul.san...@oracle.com wrote: Currently i cannot find any external uses of it.
Re: Unsafe: efficiently comparing two byte arrays
On Feb 28, 2014, at 11:29 PM, Martin Buchholz marti...@google.com wrote:
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
Yes, no choice :-) I see it only does it when unaligned access is supported:
private long getLong(long a) {
if (unaligned) {
long x = unsafe.getLong(a);
return (nativeByteOrder ? x : Bits.swap(x));
}
return Bits.getLong(a, bigEndian);
}
I suppose it could detect if the address + position also aligns, but ideally
that should be hoisted out of any loop so the unaligned pre/post bytes are
handled separately to the aligned sequence viewed as longs, which most likely
means explicit code for the comparison use-case.
Now i am in two minds to whether to add ByteBuffer.compareUnsigned or add
Arrays.compareUnsigned.
An explosion of methods on Arrays for all types (plus to/from versions) would
be annoying.
Paul.
Re: i18n dev RFR: 8037012: (tz) Support tzdata2014a
Looks good to me. regards, Sean. On 12/03/2014 15:05, Aleksej Efimov wrote: Hello, Can I have a review for a tzdata2014a [1] integration to JDK9: http://cr.openjdk.java.net/~aefimov/8037012/9/webrev.00 The following test sets were executed on the build with latest tzdata: test/sun/util/calendar test/java/util/Calendar test/sun/util/resources/TimeZone test/sun/util/calendar test/java/util/TimeZone test/java/time test/java/util/Formatter test/closed/java/util/Calendar test/closed/java/util/TimeZone One test failure was observed - see the bug report for details: [2]. The review for this test failure fix will be sent in a few moments. Other tests passes. Thank you, Aleksej [1] https://bugs.openjdk.java.net/browse/JDK-8037012 [2] https://bugs.openjdk.java.net/browse/JDK-8037180
Re: Unsafe: efficiently comparing two byte arrays
On 03/13/2014 11:57 AM, Paul Sandoz wrote: Now i am in two minds to whether to add ByteBuffer.compareUnsigned or add Arrays.compareUnsigned. An explosion of methods on Arrays for all types (plus to/from versions) would be annoying. Surely it's a no brainer? All we have to do is add the intrinsics for ByteBuffers, then we get fast ByteBuffers and fast array comparisons too. I don't see the problem. Andrew.
Re: i18n dev RFR: 8037180: [TEST_BUG] test/sun/util/calendar/zi/Zoneinfo.java incorrectly calculates raw GMT offset change time
Looks good Aleksej. A future rule change doesn't necessarily mean a new GMT offset change. Original test logic seemed buggy. regards, Sean. On 12/03/2014 15:06, Aleksej Efimov wrote: Hello, Can I have a review for a 'test/sun/util/calendar/zi/Zoneinfo.java' test bug failure [1] related to the latest tzdata2014a integration [2]. The test failure message: Asia/Istanbul : Asia/Istanbul offset=720,dstSavings=360,useDaylight=true,transitions=171,offsets=5,checksum=-508379603,gmtChanged=false [NG]offset=720,dstSavings=360,useDaylight=true,transitions=171,offsets=5,checksum=-508379603,gmtChanged=true --System.err:(13/732)-- java.lang.RuntimeException: FAILED: Asia/Istanbul at TestZoneInfo310.main(TestZoneInfo310.java:170) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.lang.reflect.Method.invoke(Method.java:484) at com.sun.javatest.regtest.MainWrapper$MainThread.run(MainWrapper.java:94) at java.lang.Thread.run(Thread.java:744) This failure caused by the following entry in test tzdata files: ZoneEurope/Istanbul 1:55:52 - LMT 1880 1:56:56 - IMT 1910 Oct # Istanbul Mean Time? 2:00Turkey EE%sT 1978 Oct 15 3:00Turkey TR%sT 1985 Apr 20 # Turkey Time 2:00Turkey EE%sT 2007 2:00EU EE%sT 2011 Mar 27 1:00u 2:00- EET 2011 Mar 28 1:00u 2:00EU EE%sT 2014 Mar 30 1:00u 2:00- EET 2014 Mar 31 1:00u 2:00EU EE%sT The raw GMT offset here is constant=2:00 and not changed since 1985 Apr 20. But the test code 'test/sun/util/calendar/zi' shows that there will be a future raw GMT offset change. The following fix resolves this problem: http://cr.openjdk.java.net/~aefimov/8037180/9/webrev.00/ -Aleksej [1] https://bugs.openjdk.java.net/browse/JDK-8037180 [2] https://bugs.openjdk.java.net/browse/JDK-8037012
Re: i18n dev RFR: 8037012: (tz) Support tzdata2014a
+1 Masayoshi On 3/13/2014 9:18 PM, Seán Coffey wrote: Looks good to me. regards, Sean. On 12/03/2014 15:05, Aleksej Efimov wrote: Hello, Can I have a review for a tzdata2014a [1] integration to JDK9: http://cr.openjdk.java.net/~aefimov/8037012/9/webrev.00 The following test sets were executed on the build with latest tzdata: test/sun/util/calendar test/java/util/Calendar test/sun/util/resources/TimeZone test/sun/util/calendar test/java/util/TimeZone test/java/time test/java/util/Formatter test/closed/java/util/Calendar test/closed/java/util/TimeZone One test failure was observed - see the bug report for details: [2]. The review for this test failure fix will be sent in a few moments. Other tests passes. Thank you, Aleksej [1] https://bugs.openjdk.java.net/browse/JDK-8037012 [2] https://bugs.openjdk.java.net/browse/JDK-8037180
Re: i18n dev RFR: 8037180: [TEST_BUG] test/sun/util/calendar/zi/Zoneinfo.java incorrectly calculates raw GMT offset change time
+1 Masayoshi On 3/13/2014 9:19 PM, Seán Coffey wrote: Looks good Aleksej. A future rule change doesn't necessarily mean a new GMT offset change. Original test logic seemed buggy. regards, Sean. On 12/03/2014 15:06, Aleksej Efimov wrote: Hello, Can I have a review for a 'test/sun/util/calendar/zi/Zoneinfo.java' test bug failure [1] related to the latest tzdata2014a integration [2]. The test failure message: Asia/Istanbul : Asia/Istanbul offset=720,dstSavings=360,useDaylight=true,transitions=171,offsets=5,checksum=-508379603,gmtChanged=false [NG]offset=720,dstSavings=360,useDaylight=true,transitions=171,offsets=5,checksum=-508379603,gmtChanged=true --System.err:(13/732)-- java.lang.RuntimeException: FAILED: Asia/Istanbul at TestZoneInfo310.main(TestZoneInfo310.java:170) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.lang.reflect.Method.invoke(Method.java:484) at com.sun.javatest.regtest.MainWrapper$MainThread.run(MainWrapper.java:94) at java.lang.Thread.run(Thread.java:744) This failure caused by the following entry in test tzdata files: ZoneEurope/Istanbul 1:55:52 - LMT 1880 1:56:56 - IMT 1910 Oct # Istanbul Mean Time? 2:00Turkey EE%sT 1978 Oct 15 3:00Turkey TR%sT 1985 Apr 20 # Turkey Time 2:00Turkey EE%sT 2007 2:00EU EE%sT 2011 Mar 27 1:00u 2:00- EET 2011 Mar 28 1:00u 2:00EU EE%sT 2014 Mar 30 1:00u 2:00- EET 2014 Mar 31 1:00u 2:00EU EE%sT The raw GMT offset here is constant=2:00 and not changed since 1985 Apr 20. But the test code 'test/sun/util/calendar/zi' shows that there will be a future raw GMT offset change. The following fix resolves this problem: http://cr.openjdk.java.net/~aefimov/8037180/9/webrev.00/ -Aleksej [1] https://bugs.openjdk.java.net/browse/JDK-8037180 [2] https://bugs.openjdk.java.net/browse/JDK-8037012
Re: Unsafe: efficiently comparing two byte arrays
On Mar 13, 2014, at 1:19 PM, Andrew Haley a...@redhat.com wrote: On 03/13/2014 11:57 AM, Paul Sandoz wrote: Now i am in two minds to whether to add ByteBuffer.compareUnsigned or add Arrays.compareUnsigned. An explosion of methods on Arrays for all types (plus to/from versions) would be annoying. Surely it's a no brainer? All we have to do is add the intrinsics for ByteBuffers, then we get fast ByteBuffers and fast array comparisons too. I don't see the problem. For byte[] comparison, I don't think optimal long access is sufficient on it's own due to alignment issues. It would be nice if we can avoid burdening users with alignment issues to ensure the main comparison loop is efficient. A quick solution is to leverage Unsafe within a ByteBuffer.compareUnsigned method. In fact i believe Unsafe could be used (as Aleksey says in [1]) for put/get as well, which i presume is likely to be much easier/quicker to implement. Might be a good first step until a more superior intrinsics solution is implemented? Paul. [1] https://bugs.openjdk.java.net/browse/JDK-8026049
Re: Unsafe: efficiently comparing two byte arrays
On 03/13/2014 12:49 PM, Paul Sandoz wrote: On Mar 13, 2014, at 1:19 PM, Andrew Haley a...@redhat.com wrote: On 03/13/2014 11:57 AM, Paul Sandoz wrote: Now i am in two minds to whether to add ByteBuffer.compareUnsigned or add Arrays.compareUnsigned. An explosion of methods on Arrays for all types (plus to/from versions) would be annoying. Surely it's a no brainer? All we have to do is add the intrinsics for ByteBuffers, then we get fast ByteBuffers and fast array comparisons too. I don't see the problem. For byte[] comparison, I don't think optimal long access is sufficient on it's own due to alignment issues. It would be nice if we can avoid burdening users with alignment issues to ensure the main comparison loop is efficient. But alignment issues are a feature of the hardware, not of software. The problem right now is that ByteBuffers use a byte-by-byte comparison of longs, even when it isn't needed, because there aren't the intrinsics. A quick solution is to leverage Unsafe within a ByteBuffer.compareUnsigned method. In fact i believe Unsafe could be used (as Aleksey says in [1]) for put/get as well, which i presume is likely to be much easier/quicker to implement. Might be a good first step until a more superior intrinsics solution is implemented? I still don't get it, sorry. What can Unsafe do that ByteBuffer intrinsics can't do? Andrew.
Accessing type annotations at runtime
Hi, Is it possible to retrieve type annotations (as defined by JSR 308) using reflection at runtime? E.g. I would like to retrieve the @NotNull annotation from a member declared like this: private List@NotNull String names; I assumed that this would be possible using reflection, but I couldn't find a way to do it. Is there an example for this somewhere? Or are type annotations only meant to be accessed by compile-time tools such as the Checker framework? I noticed that the byte code of my class contains information about the annotation, so I guess one could access it by examining the class file if it's really not possible via the reflection API. Many thanks, --Gunnar
Re: RFR: JDK9: 8037221: [asm] refresh internal ASM version
Thanks Paul, for the review!. Hi, Eye-balled the changes, nothing obvious popped out. Looks good. It's very minor, and should not block the code going back, but occured is a common misspelling of occurred. fyi, these are *all* changes made upstream, by the ASM team, the only file we generate is the version.txt to help us identify what we have. Kumar Paul. On Mar 12, 2014, at 10:24 PM, Kumar Srinivasan kumar.x.sriniva...@oracle.com wrote: Hello, Appreciate if someone cane review this change, so that I can push this into the jdk9 repository, refreshing the internal ASM sources from rev 1700 to 1721, of particular interest is Rev 1710, which fixes [1] known to cause verification errors in the VM. Thereafter I will be back-porting this changeset into 8u20. http://cr.openjdk.java.net/~ksrini/8037221/ The following regression tests have been run: nashorn/test jdk/test/tools/pack200 jdk/test/java/lang/invoke jdk/test/java/util jdk/test/javax/script jdk/test/sun/tools/jrunscript jdk/test/closed/com/oracle/jfr langtools/test Nashorn additional test targets: test test262 octane Thanks in advance Kumar [1] http://forge.ow2.org/tracker/?group_id=23atid=100023func=detailaid=316545
Re: Unsafe: efficiently comparing two byte arrays
On 03/13/2014 07:19 AM, Andrew Haley wrote: On 03/13/2014 11:57 AM, Paul Sandoz wrote: Now i am in two minds to whether to add ByteBuffer.compareUnsigned or add Arrays.compareUnsigned. An explosion of methods on Arrays for all types (plus to/from versions) would be annoying. Surely it's a no brainer? All we have to do is add the intrinsics for ByteBuffers, then we get fast ByteBuffers and fast array comparisons too. I don't see the problem. Why such a hangup on ByteBuffers and Arrays? I think it'd be super useful to simply have methods on Long, Integer, etc: public static void getBytesBE(long orig, byte[] dst, int off) public static void getBytesLE(long orig, byte[] dst, int off) public static int compareBE(long orig, byte[] other, int off) public static int compareLE(long orig, byte[] other, int off) public static long getLongFromBytesBE(byte[] src, int off) public static long getLongFromBytesLE(byte[] src, int off) ... It makes sense as there are already some byte- and bit-ish methods on there (reversing byte order for example, or finding population count, etc.). If these were intrinsic, ByteBuffer could use these methods (as could Arrays.fill(...) and friends, if one was so inclined). Then Data*Stream could use them as well perhaps. -- - DML
Re: Unsafe: efficiently comparing two byte arrays
On Mar 13, 2014, at 1:57 PM, Andrew Haley a...@redhat.com wrote:
On 03/13/2014 12:49 PM, Paul Sandoz wrote:
On Mar 13, 2014, at 1:19 PM, Andrew Haley a...@redhat.com wrote:
On 03/13/2014 11:57 AM, Paul Sandoz wrote:
Now i am in two minds to whether to add ByteBuffer.compareUnsigned or add
Arrays.compareUnsigned.
An explosion of methods on Arrays for all types (plus to/from versions)
would be annoying.
Surely it's a no brainer? All we have to do is add the intrinsics for
ByteBuffers, then we get fast ByteBuffers and fast array comparisons too.
I don't see the problem.
For byte[] comparison, I don't think optimal long access is
sufficient on it's own due to alignment issues. It would be nice if
we can avoid burdening users with alignment issues to ensure the
main comparison loop is efficient.
But alignment issues are a feature of the hardware, not of software.
The problem right now is that ByteBuffers use a byte-by-byte
comparison of longs, even when it isn't needed, because there aren't
the intrinsics.
I was considering the following case:
byte[] b1 = ...
byte[] b2 = ...
// compare from offset
ByteBuffer left = ByteBuffer.wrap(b1, 1, b1.length - 1);
ByteBuffer right = ByteBuffer.wrap(b2, 1, b2.length - 1);
int r = compare(left, right);
static int compare(ByteBuffer left, ByteBuffer right) {
int minLength = Math.min(left.remaining(), right.remaining());
int minWords = minLength / 8;
for (int i = 0; i minWords * 8; i += 8) {
long lw = left.getLong(); // Inefficient when unaligned access is
not supported by hardware?
long rw = right.getLong(); // Are bounds checks hoisted out? i
presume so
if (lw != rw) {
return Long.compareUnsigned(lw, rw);
}
}
for (int i = minWords * 8; i minLength; i++) {
int result = Byte.toUnsignedInt(left.get()) -
Byte.toUnsignedInt(right.get());
if (result != 0) {
return result;
}
}
return left.remaining() - right.remaining();
}
Certain Apache code (e.g. see Cassandra) supports comparing byte[] arrays with
offsets and i am not sure that code is portable [1].
I was wondering in the case where the hardware does not support unaligned
access it might be possible to sill optimize by reading longs at aligned
positions and doing some some extra bit twiddling.
A quick solution is to leverage Unsafe within a
ByteBuffer.compareUnsigned method. In fact i believe Unsafe could be
used (as Aleksey says in [1]) for put/get as well, which i presume
is likely to be much easier/quicker to implement. Might be a good
first step until a more superior intrinsics solution is implemented?
I still don't get it, sorry. What can Unsafe do that ByteBuffer
intrinsics can't do?
We can arguably implement it faster [2] and i presume it will be simpler too.
Paul.
[1]
https://git-wip-us.apache.org/repos/asf?p=cassandra.git;a=blob;f=src/java/org/apache/cassandra/utils/FastByteComparisons.java;h=4be6cd42b3035055c2b6102718d2cf49b73669f1;hb=HEAD#l184
[2] Well, i argue it is since i know next to nothing about how to implement an
intrinsic :-)
Re: Accessing type annotations at runtime
Hello, See the methods in java.lang.reflect named getAnnotedFoo which return java.lang.reflect.AnnotedType or a subinterface. -Joe On 3/13/2014 6:24 AM, Gunnar Morling wrote: Hi, Is it possible to retrieve type annotations (as defined by JSR 308) using reflection at runtime? E.g. I would like to retrieve the @NotNull annotation from a member declared like this: private List@NotNull String names; I assumed that this would be possible using reflection, but I couldn't find a way to do it. Is there an example for this somewhere? Or are type annotations only meant to be accessed by compile-time tools such as the Checker framework? I noticed that the byte code of my class contains information about the annotation, so I guess one could access it by examining the class file if it's really not possible via the reflection API. Many thanks, --Gunnar
RFR [8014066] Mistake in documentation of ArrayList#removeRange
Hello! Would you please review a simple fix of the javadoc for ArrayList#removeRange() method? The doc says that IndexOutOfBoundsException is thrown if fromIndex or toIndex is out of range (fromIndex 0 || fromIndex = size() || toIndex size() || toIndex fromIndex). The condition 'fromIndex = size()' isn't true and should be removed from the doc. For example, the code list.removeRange(size(), size()) does not throw any exception. BUGURL: https://bugs.openjdk.java.net/browse/JDK-8014066 WEBREV: http://cr.openjdk.java.net/~igerasim/8014066/0/webrev/ Sincerely yours, Ivan
Re: RFR [8014066] Mistake in documentation of ArrayList#removeRange
Ivan, This does look a little odd, but since fromIndex is inclusive I would think that it should throw if passed a value of size() ?? -Chris. On 13 Mar 2014, at 15:29, Ivan Gerasimov ivan.gerasi...@oracle.com wrote: Hello! Would you please review a simple fix of the javadoc for ArrayList#removeRange() method? The doc says that IndexOutOfBoundsException is thrown if fromIndex or toIndex is out of range (fromIndex 0 || fromIndex = size() || toIndex size() || toIndex fromIndex). The condition 'fromIndex = size()' isn't true and should be removed from the doc. For example, the code list.removeRange(size(), size()) does not throw any exception. BUGURL: https://bugs.openjdk.java.net/browse/JDK-8014066 WEBREV: http://cr.openjdk.java.net/~igerasim/8014066/0/webrev/ Sincerely yours, Ivan
RE: RFR: 8035584: (s) ArrayList(c) should avoid inflation if c is empty
Mike,
The constructor modification looks good. The only other alternative I can
think would be to use 'c.toArray(EMPTY_ELEMENTDATA)' to avoid creating an extra
array. I'm guessing that version would not perform as well as your current
version.
The modification for toArray violates the List contract because the returned
array must be safe (must use 'new' and must not retain reference). I think
you'll have to back out that change. Contract violation aside, the only case
that I can see that might unsafe for an empty array would be acquiring the
monitor of EMPTY_ELEMENTDATA array. When we patched the Collections$EmptyXXX
classes we avoided returning a cached empty array for this reason.
Jason
Subject: Re: RFR: 8035584: (s) ArrayList(c) should avoid inflation if c is empty
From: mike.dui...@oracle.com
Date: Wed, 12 Mar 2014 12:41:00 -0700
CC: marti...@google.com; core-libs-dev@openjdk.java.net
To: jason_mehr...@hotmail.com
Hi Jason,'
Using isEmpty() was intended to save the array creation but you make a valid
point regarding correctness. I have amended the webrev. This handling suggests
that it would useful for implementation to cache the empty result for toArray()
and I have also added this to ArrayList's toArray.
http://cr.openjdk.java.net/~mduigou/JDK-8035584/3/webrev/
Mike
On Mar 12 2014, at 07:06 , Jason Mehrens jason_mehr...@hotmail.com wrote:
Mike,
In the constructor do you think you should skip the call to isEmpty and just
check the length of toArray? Seems like since the implementation of the given
collection is unknown it is probably best to perform as little interaction with
it as possible. Also it would be possible for isEmpty to return false followed
by toArray returning a zero length array that is different from cached copies.
Jason
Subject: Re: RFR: 8035584: (s) ArrayList(c) should avoid inflation if c is
empty
From: mike.dui...@oracle.com
Date: Tue, 11 Mar 2014 18:18:26 -0700
To: marti...@google.com
CC: core-libs-dev@openjdk.java.net
On Mar 11 2014, at 17:42 , Martin Buchholz marti...@google.com wrote:
I'm hoping y'all have evidence that empty ArrayLists are common in the wild.
Yes, certainly. From the original proposal:
[This change] based upon analysis that shows that in large applications as
much as 10% of maps and lists are initialized but never receive any
entries. A smaller number spend a large proportion of their lifetime empty.
We've found similar results across other workloads as well.
It is curious that empty lists grow immediately to 10, while ArrayList with
capacity 1 grows to 2, then 3... Some people might think that a bug.
Yes, that's unfortunate. I've made another version that uses a second
sentinel for the default sized but empty case.
Now I want to reduce the ensureCapacity reallocations! It seems like insane
churn to replace the arrays that frequently.
There are more nbsp; changes that need to be reverted.
Else looks good.
- * More formally, returns the lowest index tti/tt such that
- *
tt(o==nullnbsp;?nbsp;get(i)==nullnbsp;:nbsp;o.equals(get(i)))/tt,
+ * More formally, returns the lowest index {@code i} such that
+ * {@code
(o==nullnbsp;?nbsp;get(i)==nullnbsp;:nbsp;o.equals(get(i)))},
Corrected. Thank you.
http://cr.openjdk.java.net/~mduigou/JDK-8035584/2/webrev/
Mike
On Tue, Mar 11, 2014 at 5:20 PM, Mike Duigou mike.dui...@oracle.com wrote:
I've actually always used scp. :-)
Since I accepted all of your changes as suggested and had no other changes
I was just going to go ahead and push once testing was done.
I've now prepared a revised webrev and can still accept feedback.
http://cr.openjdk.java.net/~mduigou/JDK-8035584/1/webrev/
(Note: The webrev also contains
https://bugs.openjdk.java.net/browse/JDK-8037097 since I am testing/pushing
the two issues together.)
Mike
On Mar 11 2014, at 16:42 , Martin Buchholz marti...@google.com wrote:
I don't see the updated webrev. Maybe you also fell victim to rsync to
cr no longer working?
On Tue, Mar 11, 2014 at 4:34 PM, Mike Duigou mike.dui...@oracle.com
wrote:
On Feb 21 2014, at 14:56 , Martin Buchholz marti...@google.com wrote:
You should do tt - code conversion separately, and do it pervasively
across the entire JDK.
From your lips to God's ears I keep suggesting this along with a
restyle to official style every time we create new repos. Seems unlikely
unfortunately as it makes backports harder.
This is not right.
+ * {@code
(o==nullnbsp;?nbsp;get(i)==nullnbsp;:nbsp;o.equals(get(i)))}
Corrected.
You accidentally deleted a stray space here?
-this.elementData = EMPTY_ELEMENTDATA;
+ this.elementData = EMPTY_ELEMENTDATA;
Corrected.
public ArrayList(int initialCapacity) {
-super();
if (initialCapacity 0)
throw
Re: Accessing type annotations at runtime
Hi Joe, Thanks for the reply and hinting me the right direction! I had seen these methods but was missing the down-cast to AnnotatedParameterizedType which gives access to getAnnotatedActualTypeArguments(). In case it's helpful to others, that's what I ended up with: Field myField = ...; // ListString AnnotatedParameterizedType type = (AnnotatedParameterizedType) declaredField.getAnnotatedType(); // String AnnotatedType typeArg = type.getAnnotatedActualTypeArguments()[0]; // @NotNull Annotation annotation = typeArg.getAnnotations()[0]; Thanks again, --Gunnar 2014-03-13 15:39 GMT+01:00 Joe Darcy joe.da...@oracle.com: Hello, See the methods in java.lang.reflect named getAnnotedFoo which return java.lang.reflect.AnnotedType or a subinterface. -Joe On 3/13/2014 6:24 AM, Gunnar Morling wrote: Hi, Is it possible to retrieve type annotations (as defined by JSR 308) using reflection at runtime? E.g. I would like to retrieve the @NotNull annotation from a member declared like this: private List@NotNull String names; I assumed that this would be possible using reflection, but I couldn't find a way to do it. Is there an example for this somewhere? Or are type annotations only meant to be accessed by compile-time tools such as the Checker framework? I noticed that the byte code of my class contains information about the annotation, so I guess one could access it by examining the class file if it's really not possible via the reflection API. Many thanks, --Gunnar
Re: RFR: 8035584: (s) ArrayList(c) should avoid inflation if c is empty
You could definitely safely do a check to see if elementData ==
EMPTY_ELEMENTDATA (irrespective of size), and if so, return
EMPTY_ELEMENTDATA instead of copying.
I don't think however that that method, as is, is actually unsafe. I
can't think of a code path where, say, a sudden concurrent change in
size would cause a problem that wouldn't already be a problem with the
original method. If size was 0 when the assignment and check took
place, and suddenly grew, returning an empty array is OK at this point.
If size was 0 and then suddenly shrunk or went to 0, the copy would
copy extra elements - but that would already be the case with the
original implementation.
On 03/13/2014 10:56 AM, Jason Mehrens wrote:
Mike,
The constructor modification looks good. The only other alternative I can
think would be to use 'c.toArray(EMPTY_ELEMENTDATA)' to avoid creating an extra
array. I'm guessing that version would not perform as well as your current
version.
The modification for toArray violates the List contract because the returned
array must be safe (must use 'new' and must not retain reference). I think
you'll have to back out that change. Contract violation aside, the only case
that I can see that might unsafe for an empty array would be acquiring the
monitor of EMPTY_ELEMENTDATA array. When we patched the Collections$EmptyXXX
classes we avoided returning a cached empty array for this reason.
Jason
Subject: Re: RFR: 8035584: (s) ArrayList(c) should avoid inflation if c is empty
From: mike.dui...@oracle.com
Date: Wed, 12 Mar 2014 12:41:00 -0700
CC: marti...@google.com; core-libs-dev@openjdk.java.net
To: jason_mehr...@hotmail.com
Hi Jason,'
Using isEmpty() was intended to save the array creation but you make a valid
point regarding correctness. I have amended the webrev. This handling suggests
that it would useful for implementation to cache the empty result for toArray()
and I have also added this to ArrayList's toArray.
http://cr.openjdk.java.net/~mduigou/JDK-8035584/3/webrev/
Mike
On Mar 12 2014, at 07:06 , Jason Mehrens jason_mehr...@hotmail.com wrote:
Mike,
In the constructor do you think you should skip the call to isEmpty and just
check the length of toArray? Seems like since the implementation of the given
collection is unknown it is probably best to perform as little interaction with
it as possible. Also it would be possible for isEmpty to return false followed
by toArray returning a zero length array that is different from cached copies.
Jason
Subject: Re: RFR: 8035584: (s) ArrayList(c) should avoid inflation if c is
empty
From: mike.dui...@oracle.com
Date: Tue, 11 Mar 2014 18:18:26 -0700
To: marti...@google.com
CC: core-libs-dev@openjdk.java.net
On Mar 11 2014, at 17:42 , Martin Buchholz marti...@google.com wrote:
I'm hoping y'all have evidence that empty ArrayLists are common in the wild.
Yes, certainly. From the original proposal:
[This change] based upon analysis that shows that in large applications as much
as 10% of maps and lists are initialized but never receive any entries. A
smaller number spend a large proportion of their lifetime empty. We've found
similar results across other workloads as well.
It is curious that empty lists grow immediately to 10, while ArrayList with
capacity 1 grows to 2, then 3... Some people might think that a bug.
Yes, that's unfortunate. I've made another version that uses a second sentinel
for the default sized but empty case.
Now I want to reduce the ensureCapacity reallocations! It seems like insane
churn to replace the arrays that frequently.
There are more nbsp; changes that need to be reverted.
Else looks good.
- * More formally, returns the lowest index tti/tt such that
- *
tt(o==nullnbsp;?nbsp;get(i)==nullnbsp;:nbsp;o.equals(get(i)))/tt,
+ * More formally, returns the lowest index {@code i} such that
+ * {@code (o==nullnbsp;?nbsp;get(i)==nullnbsp;:nbsp;o.equals(get(i)))},
Corrected. Thank you.
http://cr.openjdk.java.net/~mduigou/JDK-8035584/2/webrev/
Mike
On Tue, Mar 11, 2014 at 5:20 PM, Mike Duigou mike.dui...@oracle.com wrote:
I've actually always used scp. :-)
Since I accepted all of your changes as suggested and had no other changes I
was just going to go ahead and push once testing was done.
I've now prepared a revised webrev and can still accept feedback.
http://cr.openjdk.java.net/~mduigou/JDK-8035584/1/webrev/
(Note: The webrev also contains
https://bugs.openjdk.java.net/browse/JDK-8037097 since I am testing/pushing the
two issues together.)
Mike
On Mar 11 2014, at 16:42 , Martin Buchholz marti...@google.com wrote:
I don't see the updated webrev. Maybe you also fell victim to rsync to cr no
longer working?
On Tue, Mar 11, 2014 at 4:34 PM, Mike Duigou mike.dui...@oracle.com wrote:
On Feb 21 2014, at 14:56 , Martin Buchholz marti...@google.com wrote:
You should do tt - code conversion separately, and do it pervasively across
the entire
Re: RFR [8014066] Mistake in documentation of ArrayList#removeRange
Thank you Chris!
It is System.arraycopy() method, where checking is performed and the
exception is thrown.
Here's this code:
if ( (((unsigned int) length + (unsigned int) src_pos) (unsigned
int) s-length())
|| (((unsigned int) length + (unsigned int) dst_pos) (unsigned
int) d-length()) ) {
THROW(vmSymbols::java_lang_ArrayIndexOutOfBoundsException());
}
This confirms that size() is a valid value for fromIndex.
Another way to think of it is that fromIndex = toIndex, and toIndex can
be equal to size().
Therefore, fromIndex can be equal to size() too.
The documentation also says that 'If toIndex==fromIndex, this operation
has no effect.', so removeRange(size(), size()) is a valid expression
and should not cause an exception be thrown (and it actually does not).
Sincerely yours,
Ivan
On 13.03.2014 19:47, Chris Hegarty wrote:
Ivan,
This does look a little odd, but since fromIndex is inclusive I would think
that it should throw if passed a value of size() ??
-Chris.
On 13 Mar 2014, at 15:29, Ivan Gerasimov ivan.gerasi...@oracle.com wrote:
Hello!
Would you please review a simple fix of the javadoc for ArrayList#removeRange()
method?
The doc says that IndexOutOfBoundsException is thrown if fromIndex or toIndex is out of
range (fromIndex 0 || fromIndex = size() || toIndex size() || toIndex
fromIndex).
The condition 'fromIndex = size()' isn't true and should be removed from the
doc.
For example, the code list.removeRange(size(), size()) does not throw any
exception.
BUGURL: https://bugs.openjdk.java.net/browse/JDK-8014066
WEBREV: http://cr.openjdk.java.net/~igerasim/8014066/0/webrev/
Sincerely yours,
Ivan
Re: Unsafe: efficiently comparing two byte arrays
On 03/13/2014 02:19 PM, Paul Sandoz wrote:
On Mar 13, 2014, at 1:57 PM, Andrew Haley a...@redhat.com wrote:
On 03/13/2014 12:49 PM, Paul Sandoz wrote:
On Mar 13, 2014, at 1:19 PM, Andrew Haley a...@redhat.com wrote:
On 03/13/2014 11:57 AM, Paul Sandoz wrote:
Now i am in two minds to whether to add ByteBuffer.compareUnsigned or add
Arrays.compareUnsigned.
An explosion of methods on Arrays for all types (plus to/from versions)
would be annoying.
Surely it's a no brainer? All we have to do is add the intrinsics for
ByteBuffers, then we get fast ByteBuffers and fast array comparisons too.
I don't see the problem.
For byte[] comparison, I don't think optimal long access is
sufficient on it's own due to alignment issues. It would be nice if
we can avoid burdening users with alignment issues to ensure the
main comparison loop is efficient.
But alignment issues are a feature of the hardware, not of software.
The problem right now is that ByteBuffers use a byte-by-byte
comparison of longs, even when it isn't needed, because there aren't
the intrinsics.
I was considering the following case:
byte[] b1 = ...
byte[] b2 = ...
// compare from offset
ByteBuffer left = ByteBuffer.wrap(b1, 1, b1.length - 1);
ByteBuffer right = ByteBuffer.wrap(b2, 1, b2.length - 1);
int r = compare(left, right);
static int compare(ByteBuffer left, ByteBuffer right) {
int minLength = Math.min(left.remaining(), right.remaining());
int minWords = minLength / 8;
for (int i = 0; i minWords * 8; i += 8) {
long lw = left.getLong(); // Inefficient when unaligned access is
not supported by hardware?
Possibly, but HotSpot can do an awful lot with specialization when
offsets are known.
long rw = right.getLong(); // Are bounds checks hoisted out? i
presume so
Yes.
if (lw != rw) {
return Long.compareUnsigned(lw, rw);
}
}
for (int i = minWords * 8; i minLength; i++) {
int result = Byte.toUnsignedInt(left.get()) -
Byte.toUnsignedInt(right.get());
if (result != 0) {
return result;
}
}
return left.remaining() - right.remaining();
}
Certain Apache code (e.g. see Cassandra) supports comparing byte[]
arrays with offsets and i am not sure that code is portable [1].
I was wondering in the case where the hardware does not support
unaligned access it might be possible to sill optimize by reading
longs at aligned positions and doing some some extra bit twiddling.
Probably, but given that the offsets of two strings don't have to be
the same, it can get very messy.
A quick solution is to leverage Unsafe within a
ByteBuffer.compareUnsigned method. In fact i believe Unsafe could be
used (as Aleksey says in [1]) for put/get as well, which i presume
is likely to be much easier/quicker to implement. Might be a good
first step until a more superior intrinsics solution is implemented?
I still don't get it, sorry. What can Unsafe do that ByteBuffer
intrinsics can't do?
We can arguably implement it faster [2]
I doubt that very much. In fact, I would say that it is almost
certainly not true. HotSpot usually knows, for example, when both
offsets are zero and can generate better code for machines with strict
alignment.
And also, Unsafe has the same speed problems with unaligned data
that an intrinsic would have.
and i presume it will be simpler too.
And that.
But the key point is this: we want intrinsics for ByteBuffers anyway.
Andrew.
Re: Unsafe: efficiently comparing two byte arrays
On 03/13/2014 02:26 PM, David M. Lloyd wrote: On 03/13/2014 07:19 AM, Andrew Haley wrote: On 03/13/2014 11:57 AM, Paul Sandoz wrote: Now i am in two minds to whether to add ByteBuffer.compareUnsigned or add Arrays.compareUnsigned. An explosion of methods on Arrays for all types (plus to/from versions) would be annoying. Surely it's a no brainer? All we have to do is add the intrinsics for ByteBuffers, then we get fast ByteBuffers and fast array comparisons too. I don't see the problem. Why such a hangup on ByteBuffers and Arrays? I think it'd be super useful to simply have methods on Long, Integer, etc: public static void getBytesBE(long orig, byte[] dst, int off) public static void getBytesLE(long orig, byte[] dst, int off) public static int compareBE(long orig, byte[] other, int off) public static int compareLE(long orig, byte[] other, int off) public static long getLongFromBytesBE(byte[] src, int off) public static long getLongFromBytesLE(byte[] src, int off) I take your point, but this is a whole lot of new library work, both in implementation and specification, whereas we already have this in ByteBuffers, albeit with a suboptimal implementation. Andrew.
Re: RFR [8014066] Mistake in documentation of ArrayList#removeRange
Thank you Martin for your feedback!
I'll add the test with the next webrev.
Here's the test I used to check that the exception isn't thrown:
class Test extends java.util.ArrayListInteger {
public static void main(String[] args) throws Throwable {
Test list = new Test();
list.add(1);
list.removeRange(list.size(), list.size());
}
}
Sincerely yours,
Ivan
On 13.03.2014 20:34, Martin Buchholz wrote:
I notice there are zero jtreg tests for removeRange. That should be
fixed.
I notice there is a removeRange in CopyOnWriteArrayList, but it is
package-private instead of protected, which seems like an oversight.
Can Doug remember any history on that?
I notice that AbstractList.removeRange contains no @throws. That
should be fixed?
The existing @throws javadoc from CopyOnWriteArrayList seems better:
* @throws IndexOutOfBoundsException if fromIndex or toIndex out
of range
* ({@code fromIndex 0 || toIndex size() || toIndex
fromIndex})
This paragraph in AbstractList
* pThis method is called by the {@code clear} operation on this
list
* and its subLists. Overriding this method to take advantage of
* the internals of the list implementation can isubstantially/i
* improve the performance of the {@code clear} operation on this list
* and its subLists.
looks like it belongs inside the @implSpec (it's not a requirement on
subclasses)
ObTesting (a start)
import java.util.*;
public class RemoveRange {
static class PublicArrayListE extends ArrayListE {
PublicArrayList(int cap) { super(cap); }
public void removeRange(int fromIndex, int toIndex) {
super.removeRange(fromIndex, toIndex);
}
}
public static void main(String[] args) throws Throwable {
PublicArrayListString x = new PublicArrayListString(11);
for (int i = 0; i 11; i++) x.add(null);
x.removeRange(x.size(), x.size());
}
}
On Thu, Mar 13, 2014 at 8:29 AM, Ivan Gerasimov
ivan.gerasi...@oracle.com mailto:ivan.gerasi...@oracle.com wrote:
Hello!
Would you please review a simple fix of the javadoc for
ArrayList#removeRange() method?
The doc says that IndexOutOfBoundsException is thrown if fromIndex
or toIndex is out of range (fromIndex 0 || fromIndex = size()
|| toIndex size() || toIndex fromIndex).
The condition 'fromIndex = size()' isn't true and should be
removed from the doc.
For example, the code list.removeRange(size(), size()) does not
throw any exception.
BUGURL: https://bugs.openjdk.java.net/browse/JDK-8014066
WEBREV: http://cr.openjdk.java.net/~igerasim/8014066/0/webrev/
http://cr.openjdk.java.net/%7Eigerasim/8014066/0/webrev/
Sincerely yours,
Ivan
Re: Unsafe: efficiently comparing two byte arrays
On Mar 13, 2014, at 5:26 PM, Andrew Haley a...@redhat.com wrote: A quick solution is to leverage Unsafe within a ByteBuffer.compareUnsigned method. In fact i believe Unsafe could be used (as Aleksey says in [1]) for put/get as well, which i presume is likely to be much easier/quicker to implement. Might be a good first step until a more superior intrinsics solution is implemented? I still don't get it, sorry. What can Unsafe do that ByteBuffer intrinsics can't do? We can arguably implement it faster [2] I doubt that very much. In fact, I would say that it is almost certainly not true. HotSpot usually knows, for example, when both offsets are zero and can generate better code for machines with strict alignment. And also, Unsafe has the same speed problems with unaligned data that an intrinsic would have. I meant i can *implement* a comparison solution using Unsafe faster than i can implement a more general solution using intrinsics. I presume intrinsics might be of great advantage if SIMD instructions can be leveraged? if not would it be reasonable to assume that for common 0 offset case the two solutions might be similar in terms of performance? and i presume it will be simpler too. And that. But the key point is this: we want intrinsics for ByteBuffers anyway. Yeah, but i think that could take longer and there is also the Arrays 2.0 work to consider, the scope is considerably larger than providing an efficient comparison method for byte[] arrays. It is possible to tackle this simple use-case with an API tweak (ByteBuffer.compareUnsigned) and revisit the implementation if/when instincts for arrays get into 9. That is a small but useful step in the right direction. Paul.
Re: Unsafe: efficiently comparing two byte arrays
On 03/13/2014 05:02 PM, Paul Sandoz wrote: On Mar 13, 2014, at 5:26 PM, Andrew Haley a...@redhat.com wrote: A quick solution is to leverage Unsafe within a ByteBuffer.compareUnsigned method. In fact i believe Unsafe could be used (as Aleksey says in [1]) for put/get as well, which i presume is likely to be much easier/quicker to implement. Might be a good first step until a more superior intrinsics solution is implemented? I still don't get it, sorry. What can Unsafe do that ByteBuffer intrinsics can't do? We can arguably implement it faster [2] I doubt that very much. In fact, I would say that it is almost certainly not true. HotSpot usually knows, for example, when both offsets are zero and can generate better code for machines with strict alignment. And also, Unsafe has the same speed problems with unaligned data that an intrinsic would have. I meant i can *implement* a comparison solution using Unsafe faster than i can implement a more general solution using intrinsics. I see. Yes, what you said is clear, now I read it again. :-) I presume intrinsics might be of great advantage if SIMD instructions can be leveraged? if not would it be reasonable to assume that for common 0 offset case the two solutions might be similar in terms of performance? That's hard to say for sure. It might well be so, but I suspect you'll hit memor bandwidth limits in both cases. Yeah, but i think that could take longer and there is also the Arrays 2.0 work to consider, the scope is considerably larger than providing an efficient comparison method for byte[] arrays. OK, so I think I understand now: you can do what you need without having to drag in HotSpot engineers. But Unsafe only works really efficiently when it is intrinsified. It is possible to tackle this simple use-case with an API tweak (ByteBuffer.compareUnsigned) and revisit the implementation if/when instincts for arrays get into 9. That is a small but useful step in the right direction. Ok, fair point. Andrew.
Re: RFR [8014066] Mistake in documentation of ArrayList#removeRange
On 13.03.2014 20:37, Martin Buchholz wrote:
The corner case for removeRange(SIZE, SIZE) does seem rather tricky,
and it's easy for an implementation to get it wrong. All the more
reason to add tests!
It was a good idea to add a test for removeRange().
I just discovered that IOOB isn't thrown when you call removeRange(1, 0)
or removeRange(4, 0).
However, the exception is thrown when you call removeRange(5, 0).
The fix seems to become a bit more than just a doc typo fix :-)
Sincerely yours,
Ivan
On Thu, Mar 13, 2014 at 9:15 AM, Ivan Gerasimov
ivan.gerasi...@oracle.com mailto:ivan.gerasi...@oracle.com wrote:
Thank you Chris!
It is System.arraycopy() method, where checking is performed and
the exception is thrown.
Here's this code:
if ( (((unsigned int) length + (unsigned int) src_pos)
(unsigned int) s-length())
|| (((unsigned int) length + (unsigned int) dst_pos)
(unsigned int) d-length()) ) {
THROW(vmSymbols::java_lang_ArrayIndexOutOfBoundsException());
}
This confirms that size() is a valid value for fromIndex.
Another way to think of it is that fromIndex = toIndex, and
toIndex can be equal to size().
Therefore, fromIndex can be equal to size() too.
The documentation also says that 'If toIndex==fromIndex, this
operation has no effect.', so removeRange(size(), size()) is a
valid expression and should not cause an exception be thrown (and
it actually does not).
Sincerely yours,
Ivan
On 13.03.2014 19:47, Chris Hegarty wrote:
Ivan,
This does look a little odd, but since fromIndex is inclusive
I would think that it should throw if passed a value of size() ??
-Chris.
On 13 Mar 2014, at 15:29, Ivan Gerasimov
ivan.gerasi...@oracle.com mailto:ivan.gerasi...@oracle.com
wrote:
Hello!
Would you please review a simple fix of the javadoc for
ArrayList#removeRange() method?
The doc says that IndexOutOfBoundsException is thrown if
fromIndex or toIndex is out of range (fromIndex 0 ||
fromIndex = size() || toIndex size() || toIndex
fromIndex).
The condition 'fromIndex = size()' isn't true and should
be removed from the doc.
For example, the code list.removeRange(size(), size())
does not throw any exception.
BUGURL: https://bugs.openjdk.java.net/browse/JDK-8014066
WEBREV:
http://cr.openjdk.java.net/~igerasim/8014066/0/webrev/
http://cr.openjdk.java.net/%7Eigerasim/8014066/0/webrev/
Sincerely yours,
Ivan
Re: RFR [8014066] Mistake in documentation of ArrayList#removeRange
On 13.03.2014 22:16, Ivan Gerasimov wrote:
On 13.03.2014 20:37, Martin Buchholz wrote:
The corner case for removeRange(SIZE, SIZE) does seem rather tricky,
and it's easy for an implementation to get it wrong. All the more
reason to add tests!
It was a good idea to add a test for removeRange().
I just discovered that IOOB isn't thrown when you call removeRange(1,
0) or removeRange(4, 0).
However, the exception is thrown when you call removeRange(5, 0).
The fix seems to become a bit more than just a doc typo fix :-)
And when you do list.removeRange(1, 0), the list becomes longer.
Sincerely yours,
Ivan
On Thu, Mar 13, 2014 at 9:15 AM, Ivan Gerasimov
ivan.gerasi...@oracle.com mailto:ivan.gerasi...@oracle.com wrote:
Thank you Chris!
It is System.arraycopy() method, where checking is performed and
the exception is thrown.
Here's this code:
if ( (((unsigned int) length + (unsigned int) src_pos)
(unsigned int) s-length())
|| (((unsigned int) length + (unsigned int) dst_pos)
(unsigned int) d-length()) ) {
THROW(vmSymbols::java_lang_ArrayIndexOutOfBoundsException());
}
This confirms that size() is a valid value for fromIndex.
Another way to think of it is that fromIndex = toIndex, and
toIndex can be equal to size().
Therefore, fromIndex can be equal to size() too.
The documentation also says that 'If toIndex==fromIndex, this
operation has no effect.', so removeRange(size(), size()) is a
valid expression and should not cause an exception be thrown (and
it actually does not).
Sincerely yours,
Ivan
On 13.03.2014 19:47, Chris Hegarty wrote:
Ivan,
This does look a little odd, but since fromIndex is inclusive
I would think that it should throw if passed a value of
size() ??
-Chris.
On 13 Mar 2014, at 15:29, Ivan Gerasimov
ivan.gerasi...@oracle.com mailto:ivan.gerasi...@oracle.com
wrote:
Hello!
Would you please review a simple fix of the javadoc for
ArrayList#removeRange() method?
The doc says that IndexOutOfBoundsException is thrown if
fromIndex or toIndex is out of range (fromIndex 0 ||
fromIndex = size() || toIndex size() || toIndex
fromIndex).
The condition 'fromIndex = size()' isn't true and should
be removed from the doc.
For example, the code list.removeRange(size(), size())
does not throw any exception.
BUGURL: https://bugs.openjdk.java.net/browse/JDK-8014066
WEBREV:
http://cr.openjdk.java.net/~igerasim/8014066/0/webrev/
http://cr.openjdk.java.net/%7Eigerasim/8014066/0/webrev/
Sincerely yours,
Ivan
Re: RFR [8014066] Mistake in documentation of ArrayList#removeRange
Would you please take a look at the updated webrev:
http://cr.openjdk.java.net/~igerasim/8014066/1/webrev/
In addition to the javadoc correction, it includes a regression test and
a check for (fromIndex toIndex).
The last check turns out to be sufficient for removeRange() method to
agree with the javadoc.
Other checks are handled by the following System.arraycopy() call.
Sincerely yours,
Ivan
On 13.03.2014 22:27, Ivan Gerasimov wrote:
On 13.03.2014 22:16, Ivan Gerasimov wrote:
On 13.03.2014 20:37, Martin Buchholz wrote:
The corner case for removeRange(SIZE, SIZE) does seem rather tricky,
and it's easy for an implementation to get it wrong. All the more
reason to add tests!
It was a good idea to add a test for removeRange().
I just discovered that IOOB isn't thrown when you call removeRange(1,
0) or removeRange(4, 0).
However, the exception is thrown when you call removeRange(5, 0).
The fix seems to become a bit more than just a doc typo fix :-)
And when you do list.removeRange(1, 0), the list becomes longer.
Sincerely yours,
Ivan
On Thu, Mar 13, 2014 at 9:15 AM, Ivan Gerasimov
ivan.gerasi...@oracle.com mailto:ivan.gerasi...@oracle.com wrote:
Thank you Chris!
It is System.arraycopy() method, where checking is performed and
the exception is thrown.
Here's this code:
if ( (((unsigned int) length + (unsigned int) src_pos)
(unsigned int) s-length())
|| (((unsigned int) length + (unsigned int) dst_pos)
(unsigned int) d-length()) ) {
THROW(vmSymbols::java_lang_ArrayIndexOutOfBoundsException());
}
This confirms that size() is a valid value for fromIndex.
Another way to think of it is that fromIndex = toIndex, and
toIndex can be equal to size().
Therefore, fromIndex can be equal to size() too.
The documentation also says that 'If toIndex==fromIndex, this
operation has no effect.', so removeRange(size(), size()) is a
valid expression and should not cause an exception be thrown (and
it actually does not).
Sincerely yours,
Ivan
On 13.03.2014 19:47, Chris Hegarty wrote:
Ivan,
This does look a little odd, but since fromIndex is inclusive
I would think that it should throw if passed a value of
size() ??
-Chris.
On 13 Mar 2014, at 15:29, Ivan Gerasimov
ivan.gerasi...@oracle.com mailto:ivan.gerasi...@oracle.com
wrote:
Hello!
Would you please review a simple fix of the javadoc for
ArrayList#removeRange() method?
The doc says that IndexOutOfBoundsException is thrown if
fromIndex or toIndex is out of range (fromIndex 0 ||
fromIndex = size() || toIndex size() || toIndex
fromIndex).
The condition 'fromIndex = size()' isn't true and should
be removed from the doc.
For example, the code list.removeRange(size(), size())
does not throw any exception.
BUGURL: https://bugs.openjdk.java.net/browse/JDK-8014066
WEBREV:
http://cr.openjdk.java.net/~igerasim/8014066/0/webrev/
http://cr.openjdk.java.net/%7Eigerasim/8014066/0/webrev/
Sincerely yours,
Ivan
Re: Unsafe: removing the monitorEnter/monitorExit/tryMonitorEnter methods
On Mar 13, 2014, at 4:21 AM, Paul Sandoz paul.san...@oracle.com wrote: On Mar 12, 2014, at 10:23 PM, John Rose john.r.r...@oracle.com wrote: P.S. FTR, I wish we could also remove the per-object monitor from its privileged position, so not all objects are burdened by it, and other forms of lock can be switched into the existing notation on a type-by-type basis. This is obviously a long, long term wish. A starting point is a notion of this object has no monitor. Like an instance of a value type? Yes, and also a *boxed* instance of value type, yes. Like String or Integer, if I had my wish (incompatible change only for unreasonable code). Like the unenforced value-based classes of JDK 8, notably Optional. And frozen objects (JEP 169). http://download.java.net/jdk8/docs/api/java/lang/doc-files/ValueBased.html http://openjdk.java.net/jeps/169 — John Paul. Probably the keyword cannot be repurposed but would eventually be deprecated. And the Moon will reach Roche limit. – John On Mar 12, 2014, at 2:12 PM, John Rose john.r.r...@oracle.com wrote: I think that we've covered all the angles, and that we can remove it. – John On Mar 12, 2014, at 1:44 PM, Paul Sandoz paul.san...@oracle.com wrote: Currently i cannot find any external uses of it.
Re: RFR [8014066] Mistake in documentation of ArrayList#removeRange
Sorry, I forgot to incorporate changes to AbstractList.removeRange()
documentation, which you Martin had suggested.
Here's the webrev with those included:
http://cr.openjdk.java.net/~igerasim/8014066/2/webrev/
Sincerely yours,
Ivan
On 13.03.2014 23:03, Ivan Gerasimov wrote:
Would you please take a look at the updated webrev:
http://cr.openjdk.java.net/~igerasim/8014066/1/webrev/
In addition to the javadoc correction, it includes a regression test
and a check for (fromIndex toIndex).
The last check turns out to be sufficient for removeRange() method to
agree with the javadoc.
Other checks are handled by the following System.arraycopy() call.
Sincerely yours,
Ivan
On 13.03.2014 22:27, Ivan Gerasimov wrote:
On 13.03.2014 22:16, Ivan Gerasimov wrote:
On 13.03.2014 20:37, Martin Buchholz wrote:
The corner case for removeRange(SIZE, SIZE) does seem rather
tricky, and it's easy for an implementation to get it wrong. All
the more reason to add tests!
It was a good idea to add a test for removeRange().
I just discovered that IOOB isn't thrown when you call
removeRange(1, 0) or removeRange(4, 0).
However, the exception is thrown when you call removeRange(5, 0).
The fix seems to become a bit more than just a doc typo fix :-)
And when you do list.removeRange(1, 0), the list becomes longer.
Sincerely yours,
Ivan
On Thu, Mar 13, 2014 at 9:15 AM, Ivan Gerasimov
ivan.gerasi...@oracle.com mailto:ivan.gerasi...@oracle.com wrote:
Thank you Chris!
It is System.arraycopy() method, where checking is performed and
the exception is thrown.
Here's this code:
if ( (((unsigned int) length + (unsigned int) src_pos)
(unsigned int) s-length())
|| (((unsigned int) length + (unsigned int) dst_pos)
(unsigned int) d-length()) ) {
THROW(vmSymbols::java_lang_ArrayIndexOutOfBoundsException());
}
This confirms that size() is a valid value for fromIndex.
Another way to think of it is that fromIndex = toIndex, and
toIndex can be equal to size().
Therefore, fromIndex can be equal to size() too.
The documentation also says that 'If toIndex==fromIndex, this
operation has no effect.', so removeRange(size(), size()) is a
valid expression and should not cause an exception be thrown (and
it actually does not).
Sincerely yours,
Ivan
On 13.03.2014 19:47, Chris Hegarty wrote:
Ivan,
This does look a little odd, but since fromIndex is inclusive
I would think that it should throw if passed a value of
size() ??
-Chris.
On 13 Mar 2014, at 15:29, Ivan Gerasimov
ivan.gerasi...@oracle.com mailto:ivan.gerasi...@oracle.com
wrote:
Hello!
Would you please review a simple fix of the javadoc for
ArrayList#removeRange() method?
The doc says that IndexOutOfBoundsException is thrown if
fromIndex or toIndex is out of range (fromIndex 0 ||
fromIndex = size() || toIndex size() || toIndex
fromIndex).
The condition 'fromIndex = size()' isn't true and should
be removed from the doc.
For example, the code list.removeRange(size(), size())
does not throw any exception.
BUGURL: https://bugs.openjdk.java.net/browse/JDK-8014066
WEBREV:
http://cr.openjdk.java.net/~igerasim/8014066/0/webrev/
http://cr.openjdk.java.net/%7Eigerasim/8014066/0/webrev/
Sincerely yours,
Ivan
Re: RFR [8014066] Mistake in documentation of ArrayList#removeRange
On 03/13/2014 12:34 PM, Martin Buchholz wrote:
I notice there are zero jtreg tests for removeRange. That should be fixed.
I notice there is a removeRange in CopyOnWriteArrayList, but it is
package-private instead of protected, which seems like an oversight. Can Doug
remember any history on that?
CopyOnWriteArrayList does not extend AbstractList, but its
sublist does. The sublist relies on COWAL.removeRange only for clear.
So COWAL sublist clearing uses the same idea as
AbstractList, and gives it the same name, but it is not the
same AbstractList method, so need not be protected.
-Doug
I notice that AbstractList.removeRange contains no @throws. That should be
fixed?
The existing @throws javadoc from CopyOnWriteArrayList seems better:
* @throws IndexOutOfBoundsException if fromIndex or toIndex out of range
* ({@code fromIndex 0 || toIndex size() || toIndex
fromIndex})
This paragraph in AbstractList
* pThis method is called by the {@code clear} operation on this list
* and its subLists. Overriding this method to take advantage of
* the internals of the list implementation can isubstantially/i
* improve the performance of the {@code clear} operation on this list
* and its subLists.
looks like it belongs inside the @implSpec (it's not a requirement on
subclasses)
ObTesting (a start)
import java.util.*;
public class RemoveRange {
static class PublicArrayListE extends ArrayListE {
PublicArrayList(int cap) { super(cap); }
public void removeRange(int fromIndex, int toIndex) {
super.removeRange(fromIndex, toIndex);
}
}
public static void main(String[] args) throws Throwable {
PublicArrayListString x = new PublicArrayListString(11);
for (int i = 0; i 11; i++) x.add(null);
x.removeRange(x.size(), x.size());
}
}
On Thu, Mar 13, 2014 at 8:29 AM, Ivan Gerasimov ivan.gerasi...@oracle.com
mailto:ivan.gerasi...@oracle.com wrote:
Hello!
Would you please review a simple fix of the javadoc for
ArrayList#removeRange() method?
The doc says that IndexOutOfBoundsException is thrown if fromIndex or
toIndex is out of range (fromIndex 0 || fromIndex = size() || toIndex
size() || toIndex fromIndex).
The condition 'fromIndex = size()' isn't true and should be removed from
the doc.
For example, the code list.removeRange(size(), size()) does not throw any
exception.
BUGURL: https://bugs.openjdk.java.net/__browse/JDK-8014066
https://bugs.openjdk.java.net/browse/JDK-8014066
WEBREV: http://cr.openjdk.java.net/~__igerasim/8014066/0/webrev/
http://cr.openjdk.java.net/~igerasim/8014066/0/webrev/
Sincerely yours,
Ivan
Re: RFR (JAXP): 8035577: Xerces Update: impl/xpath/regex/RangeToken.java
Thanks David for the update! Joe On 3/13/2014 4:07 PM, David Li wrote: Hi, This is an update from Xerces for file impl/xpath/regex/TokenRange.java. For details, please refer to: https://bugs.openjdk.java.net/browse/JDK-8035577. Webrevs: http://cr.openjdk.java.net/~joehw/jdk9/8035577/webrev/ New test case was added for code change in RangeToken.intersectRanges. Other minor issues from last review comments are updated. Existing tests: JAXP SQE and unit tests passed. Thanks, David
Re: JDK 9 RFR of 6375303: Review use of caching in BigDecimal
On Mar 12, 2014, at 2:08 AM, Peter Levart wrote: Huh! This is a ThreadLocalRandom anti-pattern. Thou should not use a ThreadLocalRandom instance in a thread that did not obtain it via a call to ThreadLocalRandom.current()… Good point. You could create a new BigInteger(512, rnd) in the static initializer and use it to create new BigDecima(bigInteger) in testFirstToStrin. I simply replaced ThreadLocalRandom with Random which probably creates a bottleneck but there is no significant difference in the results. http://cr.openjdk.java.net/~bpb/6375303/Bench6375303.java http://cr.openjdk.java.net/~bpb/6375303/6375303-bench.html Thanks, Brian
Re: JDK 9 RFR of 6375303: Review use of caching in BigDecimal
On Mar 12, 2014, at 1:45 AM, Peter Levart wrote:
What would the following cost?
private transient String stringCache;
public String toString() {
[…]
This version did not show any significant difference in the benchmark results
(updated)
http://cr.openjdk.java.net/~bpb/6375303/6375303-bench.html
versus the other approaches. This is not to say that the benchmark is valid for
any of them. This approach does have the advantage or not using volatile.
Thanks,
Brian
Re: RFR [8014066] Mistake in documentation of ArrayList#removeRange
Hi Ivan,
I think we need to apply the brakes here and back up a bit :)
First of all these aren't typo fixes they are spec changes and they will
need to go through CCC for approval.
Second point is that the condition fromIndex = size() is already
captured by the condition if {@code fromIndex} or {@code toIndex} is
out of range. By definition fromIndex is out-of-range if it is 0 or
= size(). So I don't see any error here even if there is some redundancy.
Third, a call a.removeRange(a.size(), a.size()) hits the normal dilemma
of what should be checked first?. The spec states that
removeRange(n,n) is a no-op, so if we do that check first we just
return, even for bad things like removeRange(-5, -5). Of course if we
check all the preconditions first then we would in fact throw the IOOBE.
I'm in the camp that says we check preconditions first because it
detects silly mistakes sooner.
Fourth, your code change to add the additional pre-condition check:
protected void removeRange(int fromIndex, int toIndex) {
modCount++;
+ if (fromIndex toIndex)
+ throw new IndexOutOfBoundsException(
needs to be done _before_ the change to modCount!
And finally, the AbstractList.removeRange change:
+ * @throws NoSuchElementException if {@code (toIndex size())}
seems to reflect an implementation accident rather than a clear API
design choice. If the toIndex is out of range then
IndexOutOfBoundsException is what should be thrown. Otherwise you
constrain any subtypes that override this to throw an exception type
that only has meaning with the AbstractList implementation strategy -
and by doing so you are making ArrayList.removeRange violate this new
specification.
It is unfortunate that the existing specification(s) for removeRange are
lacking this key detail on exception processing, and lack consistency
between AbstractList and it's sublcass ArrayList. I think ArrayList has
the better/cleaner specification and that should be pushed up
AbstractList and AbstractList's implementation should be modified to
explicitly check the pre-conditions.
David
-
On 14/03/2014 5:42 AM, Ivan Gerasimov wrote:
Sorry, I forgot to incorporate changes to AbstractList.removeRange()
documentation, which you Martin had suggested.
Here's the webrev with those included:
http://cr.openjdk.java.net/~igerasim/8014066/2/webrev/
Sincerely yours,
Ivan
On 13.03.2014 23:03, Ivan Gerasimov wrote:
Would you please take a look at the updated webrev:
http://cr.openjdk.java.net/~igerasim/8014066/1/webrev/
In addition to the javadoc correction, it includes a regression test
and a check for (fromIndex toIndex).
The last check turns out to be sufficient for removeRange() method to
agree with the javadoc.
Other checks are handled by the following System.arraycopy() call.
Sincerely yours,
Ivan
On 13.03.2014 22:27, Ivan Gerasimov wrote:
On 13.03.2014 22:16, Ivan Gerasimov wrote:
On 13.03.2014 20:37, Martin Buchholz wrote:
The corner case for removeRange(SIZE, SIZE) does seem rather
tricky, and it's easy for an implementation to get it wrong. All
the more reason to add tests!
It was a good idea to add a test for removeRange().
I just discovered that IOOB isn't thrown when you call
removeRange(1, 0) or removeRange(4, 0).
However, the exception is thrown when you call removeRange(5, 0).
The fix seems to become a bit more than just a doc typo fix :-)
And when you do list.removeRange(1, 0), the list becomes longer.
Sincerely yours,
Ivan
On Thu, Mar 13, 2014 at 9:15 AM, Ivan Gerasimov
ivan.gerasi...@oracle.com mailto:ivan.gerasi...@oracle.com wrote:
Thank you Chris!
It is System.arraycopy() method, where checking is performed and
the exception is thrown.
Here's this code:
if ( (((unsigned int) length + (unsigned int) src_pos)
(unsigned int) s-length())
|| (((unsigned int) length + (unsigned int) dst_pos)
(unsigned int) d-length()) ) {
THROW(vmSymbols::java_lang_ArrayIndexOutOfBoundsException());
}
This confirms that size() is a valid value for fromIndex.
Another way to think of it is that fromIndex = toIndex, and
toIndex can be equal to size().
Therefore, fromIndex can be equal to size() too.
The documentation also says that 'If toIndex==fromIndex, this
operation has no effect.', so removeRange(size(), size()) is a
valid expression and should not cause an exception be thrown (and
it actually does not).
Sincerely yours,
Ivan
On 13.03.2014 19:47, Chris Hegarty wrote:
Ivan,
This does look a little odd, but since fromIndex is inclusive
I would think that it should throw if passed a value of
size() ??
-Chris.
On 13 Mar 2014, at 15:29, Ivan Gerasimov
ivan.gerasi...@oracle.com mailto:ivan.gerasi...@oracle.com
wrote:
Hello!
Would you please review a simple fix of the javadoc for
