Re: [JGIT PATCH 03/15] Add IntList as a more efficient representation of List<Integer>

4 messages, 2 authors, 2016-06-15 · open the first message on its own page

Re: [JGIT PATCH 03/15] Add IntList as a more efficient representation of List<Integer>

From: Shawn O. Pearce <hidden>
Date: 2016-06-15 22:45:46

Sverre Rabbelier [off-list ref] wrote:
On Fri, Dec 12, 2008 at 03:46, Shawn O. Pearce [off-list ref] wrote:
quoted
+       public String toString() {
+               final StringBuilder r = new StringBuilder();
+               r.append('[');
+               for (int i = 0; i < count; i++) {
+                       if (i > 0)
+                               r.append(", ");
+                       r.append(entries[i]);
+               }
+               r.append(']');
+               return r.toString();
+       }
+}
If you care about speed in your toString at all, pull the if statement
out of there. A friend of mine did a small benchmark once, and it was
_a lot_ slower to do the if in the for loop. I reckon you don't
though, but just in case ;).
Hmm, yea, good point.  But I don't care too much about the toString()
in this case, its meant as a debugging aid and not something one
would rely upon.  Hence I didn't think it was worth testing for the
empty list, writing the first entry, then doing a loop for [1,count).

-- 
Shawn.

Re: [JGIT PATCH 03/15] Add IntList as a more efficient representation of List<Integer>

From: Sverre Rabbelier <hidden>
Date: 2016-06-15 22:45:46

On Fri, Dec 12, 2008 at 16:15, Shawn O. Pearce [off-list ref] wrote:
Hmm, yea, good point.  But I don't care too much about the toString()
in this case, its meant as a debugging aid and not something one
would rely upon.  Hence I didn't think it was worth testing for the
empty list, writing the first entry, then doing a loop for [1,count).
Fair enough :).

-- 
Cheers,

Sverre Rabbelier

Re: [JGIT PATCH 03/15] Add IntList as a more efficient representation of List<Integer>

From: Shawn O. Pearce <hidden>
Date: 2016-06-15 22:45:46

Sverre Rabbelier [off-list ref] wrote:
On Fri, Dec 12, 2008 at 16:15, Shawn O. Pearce [off-list ref] wrote:
quoted
Hmm, yea, good point.  But I don't care too much about the toString()
in this case, its meant as a debugging aid and not something one
would rely upon.  Hence I didn't think it was worth testing for the
empty list, writing the first entry, then doing a loop for [1,count).
Fair enough :).
If you'd like to send a patch to change it, I'll apply it.  But I
don't think its worth my time to make this toString() more efficient.

Other areas of JGit I do try to micro-optimize, because they are
right smack in the middle of the critical paths.

E.g. look at ObjectId.equals(byte[],int,byte[],int). I hand-unrolled
the memcmp loop because the JIT on x86 does *soooo* much better
when the code is spelled out:

	public static boolean equals(final byte[] firstBuffer, final int fi,
			final byte[] secondBuffer, final int si) {
		return firstBuffer[fi] == secondBuffer[si]
				&& firstBuffer[fi + 1] == secondBuffer[si + 1]
				&& firstBuffer[fi + 2] == secondBuffer[si + 2]
				&& firstBuffer[fi + 3] == secondBuffer[si + 3]
				&& firstBuffer[fi + 4] == secondBuffer[si + 4]
				&& firstBuffer[fi + 5] == secondBuffer[si + 5]
				&& firstBuffer[fi + 6] == secondBuffer[si + 6]
				&& firstBuffer[fi + 7] == secondBuffer[si + 7]
				&& firstBuffer[fi + 8] == secondBuffer[si + 8]
				&& firstBuffer[fi + 9] == secondBuffer[si + 9]
				&& firstBuffer[fi + 10] == secondBuffer[si + 10]
				&& firstBuffer[fi + 11] == secondBuffer[si + 11]
				&& firstBuffer[fi + 12] == secondBuffer[si + 12]
				&& firstBuffer[fi + 13] == secondBuffer[si + 13]
				&& firstBuffer[fi + 14] == secondBuffer[si + 14]
				&& firstBuffer[fi + 15] == secondBuffer[si + 15]
				&& firstBuffer[fi + 16] == secondBuffer[si + 16]
				&& firstBuffer[fi + 17] == secondBuffer[si + 17]
				&& firstBuffer[fi + 18] == secondBuffer[si + 18]
				&& firstBuffer[fi + 19] == secondBuffer[si + 19];
	}

This block is in the critical path for any tree diff code, in
particular for a "git log -- a/" sort of operation.  Its used
to compare the SHA-1s from two different tree records to see if
they differ.  Not unrolling this was a huge penalty.

-- 
Shawn.

Re: [JGIT PATCH 03/15] Add IntList as a more efficient representation of List<Integer>

From: Sverre Rabbelier <hidden>
Date: 2016-06-15 22:45:46

On Fri, Dec 12, 2008 at 16:41, Shawn O. Pearce [off-list ref] wrote:
If you'd like to send a patch to change it, I'll apply it.  But I
don't think its worth my time to make this toString() more efficient.
I mainly mentioned it because it's in a Class meant to be more optimal
than what Java ships with, but I agree with your reasoning that this
toString is not part of what needs to be optimized.
Other areas of JGit I do try to micro-optimize, because they are
right smack in the middle of the critical paths.
Hehe, I very much agree with not optimizing prematurely, and if you do
optimize to go for it all the way.
E.g. look at ObjectId.equals(byte[],int,byte[],int). I hand-unrolled
the memcmp loop because the JIT on x86 does *soooo* much better
when the code is spelled out:
<code snipped>

Kind of sad that you have to write this kind of code if you want good
performance, ah well, perhaps someday... (import java.lang.optimized
;) ).
This block is in the critical path for any tree diff code, in
particular for a "git log -- a/" sort of operation.  Its used
to compare the SHA-1s from two different tree records to see if
they differ.  Not unrolling this was a huge penalty.
I reckon that is done a lot :). Ashame the JRE can't do that kind of
optimization for you. e.g., if you do:
for(int i = 0; i < constant; i++) {
  some_code;
}

-- 
Cheers,

Sverre Rabbelier
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help