[GitHub] qpid-proton-j pull request #20: PROTON-1965 Optimize CompositeReadableBuffer...
Github user asfgit closed the pull request at: https://github.com/apache/qpid-proton-j/pull/20 --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton-j pull request #20: PROTON-1965 Optimize CompositeReadableBuffer...
Github user tabish121 commented on a diff in the pull request:
https://github.com/apache/qpid-proton-j/pull/20#discussion_r234811453
--- Diff:
proton-j/src/main/java/org/apache/qpid/proton/codec/CompositeReadableBuffer.java
---
@@ -834,22 +834,39 @@ public boolean equals(Object other) {
return false;
}
-ReadableBuffer buffer = (ReadableBuffer)other;
-if (this.remaining() != buffer.remaining()) {
+ReadableBuffer buffer = (ReadableBuffer) other;
+final int remaining = remaining();
+if (remaining != buffer.remaining()) {
return false;
}
+if (hasArray()) {
+return equals(currentArray, position, remaining, buffer);
--- End diff --
For the single array case, when you have a slice that you've created from a
CRB that is not starting from zero then the current offset will be non-zero but
position would be so the equals check should be starting at current offset and
moving forward however many elements the limit allows for.
The position and the offset work together to control where zero is in the
buffer, you can do a get for some index in the buffer and the position controls
how far back you can rewind the offset until you've hit "zero" which might not
be zero in the array.
---
-
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton-j pull request #20: PROTON-1965 Optimize CompositeReadableBuffer...
Github user franz1981 commented on a diff in the pull request:
https://github.com/apache/qpid-proton-j/pull/20#discussion_r234719950
--- Diff:
proton-j/src/main/java/org/apache/qpid/proton/codec/CompositeReadableBuffer.java
---
@@ -834,22 +834,39 @@ public boolean equals(Object other) {
return false;
}
-ReadableBuffer buffer = (ReadableBuffer)other;
-if (this.remaining() != buffer.remaining()) {
+ReadableBuffer buffer = (ReadableBuffer) other;
+final int remaining = remaining();
+if (remaining != buffer.remaining()) {
return false;
}
+if (hasArray()) {
+return equals(currentArray, position, remaining, buffer);
--- End diff --
So ti should take into account the arrayOffset and do not rely on position?
---
-
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton-j pull request #20: PROTON-1965 Optimize CompositeReadableBuffer...
Github user gemmellr commented on a diff in the pull request:
https://github.com/apache/qpid-proton-j/pull/20#discussion_r234714952
--- Diff:
proton-j/src/main/java/org/apache/qpid/proton/codec/CompositeReadableBuffer.java
---
@@ -834,22 +834,39 @@ public boolean equals(Object other) {
return false;
}
-ReadableBuffer buffer = (ReadableBuffer)other;
-if (this.remaining() != buffer.remaining()) {
+ReadableBuffer buffer = (ReadableBuffer) other;
+final int remaining = remaining();
+if (remaining != buffer.remaining()) {
return false;
}
+if (hasArray()) {
+return equals(currentArray, position, remaining, buffer);
--- End diff --
This bit is a little broken as it uses position without account for any
array offset in the current buffer. Its position in the buffer may not be the
same as its position within the array, e.g. if it were previously sliced from a
larger buffer. Calling equals with slices from two equal buffers with different
current positions will fail.
---
-
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton-j pull request #20: PROTON-1965 Optimize CompositeReadableBuffer...
Github user franz1981 commented on a diff in the pull request:
https://github.com/apache/qpid-proton-j/pull/20#discussion_r234421816
--- Diff:
proton-j/src/main/java/org/apache/qpid/proton/codec/CompositeReadableBuffer.java
---
@@ -825,33 +825,67 @@ public int hashCode() {
}
@Override
-public boolean equals(Object other) {
-if (this == other) {
+public boolean equals(Object other)
+{
+if (this == other)
+{
return true;
}
-if (!(other instanceof ReadableBuffer)) {
+if (!(other instanceof ReadableBuffer))
+{
return false;
}
-ReadableBuffer buffer = (ReadableBuffer)other;
-if (this.remaining() != buffer.remaining()) {
+ReadableBuffer buffer = (ReadableBuffer) other;
+final int remaining = remaining();
+if (remaining != buffer.remaining())
+{
return false;
}
+if (hasArray())
+{
+return equals(currentArray, position, remaining, buffer);
+}
+else
+{
+return equals(this, buffer);
+}
+}
-final int currentPos = position();
-
-for (int i = buffer.position(); hasRemaining(); i++) {
-if (!equals(this.get(), buffer.get(i))) {
+private static boolean equals(byte[] buffer, int start, int length,
ReadableBuffer other)
+{
+final int position = other.position();
+for (int i = 0; i < length; i++)
+{
+if (buffer[start + i] != other.get(position + i))
+{
return false;
}
}
-
-position(currentPos);
-
return true;
}
+private static boolean equals(ReadableBuffer buffer, ReadableBuffer
other)
+{
+final int currentPos = buffer.position();
+try
+{
+for (int i = other.position(); buffer.hasRemaining(); i++)
+{
+if (!equals(buffer.get(), other.get(i)))
+{
+return false;
+}
+}
+return true;
+}
+finally
+{
+buffer.position(currentPos);
--- End diff --
@tabish121 I have added a reset of the position that wasn't present in the
original code, I hope that's correct
---
-
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton-j pull request #20: PROTON-1965 Optimize CompositeReadableBuffer...
GitHub user franz1981 opened a pull request: https://github.com/apache/qpid-proton-j/pull/20 PROTON-1965 Optimize CompositeReadableBuffer::equals with single chunk Using the single chunk directly while performing the byte comparison increase the performance. Master: ``` Benchmark (chunks) (direct) (size) Mode Cnt ScoreError Units CompositeReadableBufferBenchmark.equalsToByteBufferReader 1 false 8 avgt518.127 ± 1.568 ns/op CompositeReadableBufferBenchmark.equalsToByteBufferReader 1 false 64 avgt540.725 ± 2.329 ns/op CompositeReadableBufferBenchmark.equalsToByteBufferReader 1 false1024 avgt5 460.801 ± 13.786 ns/op ``` This PR: ``` Benchmark (chunks) (direct) (size) Mode Cnt Score Error Units CompositeReadableBufferBenchmark.equalsToByteBufferReader 1 false 8 avgt527.799 ± 0.462 ns/op CompositeReadableBufferBenchmark.equalsToByteBufferReader 1 false 64 avgt5 108.563 ± 1.236 ns/op CompositeReadableBufferBenchmark.equalsToByteBufferReader 1 false1024 avgt5 5251.102 ± 261.561 ns/op ``` You can merge this pull request into a Git repository by running: $ git pull https://github.com/franz1981/qpid-proton-j PROTON-1965 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/qpid-proton-j/pull/20.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #20 commit d4932b415fa73ff3378ca19af46c51fcf15d84de Author: Francesco Nigro Date: 2018-11-17T18:59:12Z PROTON-1965 Optimize CompositeReadableBuffer::equals with single chunk --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
