Hi Martin!
On 13.05.2015 0:22, Martin Buchholz wrote:
All your changes look good. But:
---
Not your bug, but it looks like the below should instead be:
throw new StringIndexOutOfBoundsException(beginIndex);
Perhaps fix in a follow-on change.
1935 if (subLen 0) {
1936
On 14.05.2015 2:06, Vitaly Davidovich wrote:
Why not look at the generated asm and not guess? :) The branch
avoiding versions may cause data dependence hazards whereas the
branchy one just has branches but assuming perfectly predicted (and
microbenchmarks typically are) can pipeline
On Wed, May 13, 2015 at 2:25 PM, Ivan Gerasimov ivan.gerasi...@oracle.com
wrote:
Benchmark Mode Cnt Score Error Units
MyBenchmark.testMethod_1 thrpt 60 1132911599.680 ± 42375177.640 ops/s
MyBenchmark.testMethod_2 thrpt 60 813737659.576 ±
On Wed, May 13, 2015 at 4:06 PM, Vitaly Davidovich vita...@gmail.com
wrote:
:) The branch avoiding versions may cause data dependence hazards whereas
the branchy one just has branches but assuming perfectly predicted (and
microbenchmarks typically are) can pipeline through. Ivan, could you
Yes, that should be the general rule of thumb for code targeting out of
order chips. The caveat is that microbenchmarks have the advantage of
being the only (typically very small) code running on the cpu, and will get
full use of execution resources; specifically in this case, it's very
likely
Need JIT generated assembly, not bytecode :). That will tell you at least
which optimizations JIT applied, how it register allocated things, etc. If
nothing obvious there, see my other reply regarding cpu event based
profiling. I'm sure Aleksey Shipilev could help out if you're really
inclined
I'd add And always look at generated asm for these types of constructs.
There's really nothing to profile in terms of | outcome unless JIT starts
tracking bit position values, and it's a cheap instruction on its own. I'd
be surprised if conditional moves are emitted here since these branches
By the way, this is pure speculation on my part by just looking at the
code. To truly find out, at least say for Intel, you'd have to run these
benchmarks under a cpu event profiler and see what it tells you for IPC,
branch mispredictions, the various stalls, etc. JMH has perfasm I believe
which
Why not look at the generated asm and not guess? :) The branch avoiding
versions may cause data dependence hazards whereas the branchy one just has
branches but assuming perfectly predicted (and microbenchmarks typically
are) can pipeline through. Ivan, could you please post the asm here?
On May 13, 2015, at 2:25 PM, Ivan Gerasimov ivan.gerasi...@oracle.com wrote:
My third concern is this: Wouldn't it be possible to implement this type of
optimization at jvm level?
At least some conditions can automatically be combined into one.
Given the information about which execution
On May 13, 2015, at 4:46 PM, Vitaly Davidovich vita...@gmail.com wrote:
I'd add And always look at generated asm for these types of constructs.
+1, with caveats about micro-versions changing stuff at that level
Hi Ivan,
The code below looks wrong to me - sb.length() resolves to sb.count, not
v2.length.
If I'm correct, then there's a missing test to be added, since this error
should be caught by some test.
private boolean nonSyncContentEquals(AbstractStringBuilder sb) {
-char v1[] = value;
On 12.05.2015 20:34, Martin Buchholz wrote:
Hi Ivan,
The code below looks wrong to me - sb.length() resolves to sb.count,
not v2.length.
If I'm correct, then there's a missing test to be added, since this
error should be caught by some test.
private boolean
All your changes look good. But:
---
Not your bug, but it looks like the below should instead be:
throw new StringIndexOutOfBoundsException(beginIndex);
Perhaps fix in a follow-on change.
1935 if (subLen 0) {
1936 throw new StringIndexOutOfBoundsException(subLen);
---
We
I have to take over this fix.
The latest webrev from the review thread above (with a few minor
changes) is here:
http://cr.openjdk.java.net/~igerasim/8071571/00/webrev/
Would you please review/approve the fix at your convenience?
Sincerely yours,
Ivan
On Mar 27, 2015, at 3:01 PM, Martin Buchholz marti...@google.com wrote:
final Type foo = this.foo;
There are IMO a few places where local finals pay for themselves,
even after Java 8.
One is Martin's idiom. The key thing to observe is that 'foo' is
a read-only snapshot or view of 'this.foo'.
Martin,
So I've updated webrev w/o adding final everywhere:
http://cr.openjdk.java.net/~lpriima/8071571/3/ .
Lev
On 03/28/2015 01:15 AM, Lev Priima wrote:
On 03/28/2015 12:59 AM, Martin Buchholz wrote:
I was only suggesting using final in the stylized
final Type foo = this.foo;
Using
On 03/27/2015 10:59 PM, Martin Buchholz wrote:
[...]
OTOH, if you touch code that uses the denigrated
char foo[]
please change it to
char[] foo
This is yet another of those global changes to the entire jdk sources that
nobody ever gets around to.
on the same subject, the Java grammar
On 03/27/2015 11:01 PM, Martin Buchholz wrote:
On Fri, Mar 27, 2015 at 2:57 PM, Roger Riggs roger.ri...@oracle.com wrote:
Hi,
@Martin: does the final have some functional reason to be present?
I see it just bulks up the source, reducing readability. If it is useful
to improve performance
Please review small cleanup in java.lang.String:
Issue: https://bugs.openjdk.java.net/browse/JDK-8071571
Webrev: http://cr.openjdk.java.net/~lpriima/8071571/0/webrev/
46 tests from jdk9/dev/jdk/test/java/lang/String* passed locally.
--
Lev
Hi Ivan,
Thanks! Agree. Updated:
http://cr.openjdk.java.net/~lpriima/8071571/1/webrev/
Lev
On 03/27/2015 06:17 PM, Ivan Gerasimov wrote:
Hi Lev!
Why don't you want to also simplify String#trim()?
-return ((st 0) || (len value.length)) ? substring(st,
len) : this;
+
On Fri, Mar 27, 2015 at 1:49 PM, Lev Priima lev.pri...@oracle.com wrote:
Martin,
You mean it should be like this:
char[] val = value;/* avoid getfield opcode */
int end = val.length;
?
Yes.
(although I personally like to write it like this:
final char[] value =
Here you copy the field into a local, but then don't make use of it to grab
the length.
2888 int len = value.length;
2889 int st = 0;
2890 char[] val = value;/* avoid getfield opcode */
Also, 'beg' and 'end' would be much better names for the locals 'st' and
'len'.
Martin,
You mean it should be like this:
char[] val = value;/* avoid getfield opcode */
int end = val.length;
?
Lev
On 03/27/2015 11:37 PM, Martin Buchholz wrote:
Here you copy the field into a local, but then don't make use of it to
grab the length.
2888 int len
Updated and put some more final in other(not @Deprecated) methods:
http://cr.openjdk.java.net/~lpriima/8071571/2/
Lev
On 03/27/2015 11:50 PM, Martin Buchholz wrote:
On Fri, Mar 27, 2015 at 1:49 PM, Lev Priima lev.pri...@oracle.com
mailto:lev.pri...@oracle.com wrote:
Martin,
You
I'm closer to Martins opinion. In my opinion, Immutability really
improves readability of code, but longer declaration doesn't. That's why
some contemporary languages use shorter var/val for declarations.
Lev
On 03/28/2015 01:01 AM, Martin Buchholz wrote:
On Fri, Mar 27, 2015 at 2:57 PM,
Hi,
@Martin: does the final have some functional reason to be present?
I see it just bulks up the source, reducing readability. If it is
useful to improve performance then fine.
Roger
On 3/27/2015 5:51 PM, Lev Priima wrote:
Updated and put some more final in other(not @Deprecated)
I was only suggesting using final in the stylized
final Type foo = this.foo;
Using final for other local variables is going further than most jdk
maintainers (including myself) go.
OTOH, if you touch code that uses the denigrated
char foo[]
please change it to
char[] foo
This is yet another
On Fri, Mar 27, 2015 at 2:57 PM, Roger Riggs roger.ri...@oracle.com wrote:
Hi,
@Martin: does the final have some functional reason to be present?
I see it just bulks up the source, reducing readability. If it is useful
to improve performance then fine.
If you use the idiom
final Type foo
29 matches
Mail list logo
