Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Avoid boxing in Framing #1247

Merged
merged 9 commits into from
May 6, 2024
Merged

Avoid boxing in Framing #1247

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
70bbeba
Fix FramingBenchmark
JD557 Apr 3, 2024
18c5db1
Add specialized indexOfByte
JD557 Apr 3, 2024
37a532f
Rename indexOfByte to indexOf
JD557 Apr 3, 2024
44cf4bc
Add missing @since(1.1.0) and reformat scaladoc
JD557 Apr 4, 2024
407de55
Fix Scala 2.12 ambiguity problem
JD557 Apr 5, 2024
e923cba
Inline nextString
JD557 Apr 8, 2024
ca20244
Change default ByteString#indexOf to call indexWhere
JD557 Apr 15, 2024
2cce3ea
Remove default indexOf override
JD557 Apr 15, 2024
19fb7af
Fix MiMa issues
JD557 Apr 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 76 additions & 0 deletions actor-tests/src/test/scala/org/apache/pekko/util/ByteStringSpec.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -717,6 +717,82 @@ class ByteStringSpec extends AnyWordSpec with Matchers with Checkers {
compact.indexOf('g', 5) should ===(5)
compact.indexOf('g', 6) should ===(-1)
}
"indexOf (specialized)" in {
He-Pin marked this conversation as resolved.
Show resolved Hide resolved
ByteString.empty.indexOf(5.toByte) should ===(-1)
val byteString1 = ByteString1.fromString("abc")
byteString1.indexOf('a'.toByte) should ===(0)
byteString1.indexOf('b'.toByte) should ===(1)
byteString1.indexOf('c'.toByte) should ===(2)
byteString1.indexOf('d'.toByte) should ===(-1)

val byteStrings = ByteStrings(ByteString1.fromString("abc"), ByteString1.fromString("efg"))
byteStrings.indexOf('a'.toByte) should ===(0)
byteStrings.indexOf('c'.toByte) should ===(2)
byteStrings.indexOf('d'.toByte) should ===(-1)
byteStrings.indexOf('e'.toByte) should ===(3)
byteStrings.indexOf('f'.toByte) should ===(4)
byteStrings.indexOf('g'.toByte) should ===(5)

val compact = byteStrings.compact
compact.indexOf('a'.toByte) should ===(0)
compact.indexOf('c'.toByte) should ===(2)
compact.indexOf('d'.toByte) should ===(-1)
compact.indexOf('e'.toByte) should ===(3)
compact.indexOf('f'.toByte) should ===(4)
compact.indexOf('g'.toByte) should ===(5)

}
"indexOf (specialized) from offset" in {
ByteString.empty.indexOf(5.toByte, -1) should ===(-1)
ByteString.empty.indexOf(5.toByte, 0) should ===(-1)
ByteString.empty.indexOf(5.toByte, 1) should ===(-1)
val byteString1 = ByteString1.fromString("abc")
byteString1.indexOf('d'.toByte, -1) should ===(-1)
byteString1.indexOf('d'.toByte, 0) should ===(-1)
byteString1.indexOf('d'.toByte, 1) should ===(-1)
byteString1.indexOf('d'.toByte, 4) should ===(-1)
byteString1.indexOf('a'.toByte, -1) should ===(0)
byteString1.indexOf('a'.toByte, 0) should ===(0)
byteString1.indexOf('a'.toByte, 1) should ===(-1)

val byteStrings = ByteStrings(ByteString1.fromString("abc"), ByteString1.fromString("efg"))
byteStrings.indexOf('c'.toByte, -1) should ===(2)
byteStrings.indexOf('c'.toByte, 0) should ===(2)
byteStrings.indexOf('c'.toByte, 2) should ===(2)
byteStrings.indexOf('c'.toByte, 3) should ===(-1)

byteStrings.indexOf('e'.toByte, -1) should ===(3)
byteStrings.indexOf('e'.toByte, 0) should ===(3)
byteStrings.indexOf('e'.toByte, 1) should ===(3)
byteStrings.indexOf('e'.toByte, 4) should ===(-1)
byteStrings.indexOf('e'.toByte, 6) should ===(-1)

byteStrings.indexOf('g'.toByte, -1) should ===(5)
byteStrings.indexOf('g'.toByte, 0) should ===(5)
byteStrings.indexOf('g'.toByte, 1) should ===(5)
byteStrings.indexOf('g'.toByte, 4) should ===(5)
byteStrings.indexOf('g'.toByte, 5) should ===(5)
byteStrings.indexOf('g'.toByte, 6) should ===(-1)

val compact = byteStrings.compact
compact.indexOf('c'.toByte, -1) should ===(2)
compact.indexOf('c'.toByte, 0) should ===(2)
compact.indexOf('c'.toByte, 2) should ===(2)
compact.indexOf('c'.toByte, 3) should ===(-1)

compact.indexOf('e'.toByte, -1) should ===(3)
compact.indexOf('e'.toByte, 0) should ===(3)
compact.indexOf('e'.toByte, 1) should ===(3)
compact.indexOf('e'.toByte, 4) should ===(-1)
compact.indexOf('e'.toByte, 6) should ===(-1)

compact.indexOf('g'.toByte, -1) should ===(5)
compact.indexOf('g'.toByte, 0) should ===(5)
compact.indexOf('g'.toByte, 1) should ===(5)
compact.indexOf('g'.toByte, 4) should ===(5)
compact.indexOf('g'.toByte, 5) should ===(5)
compact.indexOf('g'.toByte, 6) should ===(-1)
}
"copyToArray" in {
val byteString = ByteString(1, 2) ++ ByteString(3) ++ ByteString(4)

Expand Down
86 changes: 83 additions & 3 deletions actor/src/main/scala-2.12/org/apache/pekko/util/ByteString.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,20 @@ object ByteString {
}
}

override def indexOf(elem: Byte): Int = indexOf(elem, 0)
override def indexOf(elem: Byte, from: Int): Int = {
if (from >= length) -1
else {
var found = -1
var i = math.max(from, 0)
while (i < length && found == -1) {
if (bytes(i) == elem) found = i
i += 1
}
found
}
}

override def slice(from: Int, until: Int): ByteString =
if (from <= 0 && until >= length) this
else if (from >= length || until <= 0 || from >= until) ByteString.empty
Expand Down Expand Up @@ -433,6 +447,20 @@ object ByteString {
}
}

override def indexOf(elem: Byte): Int = indexOf(elem, 0)
override def indexOf(elem: Byte, from: Int): Int = {
if (from >= length) -1
else {
var found = -1
var i = math.max(from, 0)
while (i < length && found == -1) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think if we swap with found == -1 && i < length) which can reduce a bit when we just found:)

if (bytes(startIndex + i) == elem) found = i
i += 1
}
found
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how about we extract the startIndex + 1 to the start of the loop and returning a found - startIndex？

}
}

protected def writeReplace(): AnyRef = new SerializationProxy(this)

override def toArrayUnsafe(): Array[Byte] = {
Expand Down Expand Up @@ -682,8 +710,34 @@ object ByteString {
} else {
val subIndexOf = bs.indexOf(elem, relativeIndex)
if (subIndexOf < 0) {
val nextString = bsIdx + 1
find(nextString, relativeIndex - bs.length, bytesPassed + bs.length)
find(bsIdx + 1, relativeIndex - bs.length, bytesPassed + bs.length)
} else subIndexOf + bytesPassed
}
}
}

find(0, math.max(from, 0), 0)
}
}

override def indexOf(elem: Byte): Int = indexOf(elem, 0)
override def indexOf(elem: Byte, from: Int): Int = {
if (from >= length) -1
else {
val byteStringsSize = bytestrings.size

@tailrec
def find(bsIdx: Int, relativeIndex: Int, bytesPassed: Int): Int = {
if (bsIdx >= byteStringsSize) -1
else {
val bs = bytestrings(bsIdx)

if (bs.length <= relativeIndex) {
find(bsIdx + 1, relativeIndex - bs.length, bytesPassed + bs.length)
} else {
val subIndexOf = bs.indexOf(elem, relativeIndex)
if (subIndexOf < 0) {
find(bsIdx + 1, relativeIndex - bs.length, bytesPassed + bs.length)
} else subIndexOf + bytesPassed
}
}
Expand Down Expand Up @@ -788,7 +842,33 @@ sealed abstract class ByteString extends IndexedSeq[Byte] with IndexedSeqOptimiz
override def indexWhere(p: Byte => Boolean): Int = iterator.indexWhere(p)

// optimized in subclasses
override def indexOf[B >: Byte](elem: B): Int = indexOf(elem, 0)
override def indexOf[B >: Byte](elem: B): Int = indexOf[B](elem, 0)

// optimized version of indexOf for bytes, implemented in subclasses
/**
* Finds index of first occurrence of some byte in this ByteString after or at some start index.
*
* Similar to indexOf, but it avoids boxing if the value is already a byte.
*
* @param elem the element value to search for.
* @param from the start index
* @return the index `>= from` of the first element of this ByteString that is equal (as determined by `==`)
* to `elem`, or `-1`, if none exists.
* @since 1.1.0
*/
def indexOf(elem: Byte, from: Int): Int = indexOf[Byte](elem, from)

/**
* Finds index of first occurrence of some byte in this ByteString.
*
* Similar to indexOf, but it avoids boxing if the value is already a byte.
*
* @param elem the element value to search for.
* @return the index `>= from` of the first element of this ByteString that is equal (as determined by `==`)
* to `elem`, or `-1`, if none exists.
* @since 1.1.0
*/
def indexOf(elem: Byte): Int = indexOf(elem, 0)
He-Pin marked this conversation as resolved.
Show resolved Hide resolved

override def grouped(size: Int): Iterator[ByteString] = {
if (size <= 0) {
Expand Down
82 changes: 78 additions & 4 deletions actor/src/main/scala-2.13/org/apache/pekko/util/ByteString.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,19 @@ object ByteString {
}
}

override def indexOf(elem: Byte, from: Int): Int = {
if (from >= length) -1
else {
var found = -1
var i = math.max(from, 0)
while (i < length && found == -1) {
if (bytes(i) == elem) found = i
i += 1
}
found
}
}

override def slice(from: Int, until: Int): ByteString =
if (from <= 0 && until >= length) this
else if (from >= length || until <= 0 || from >= until) ByteString.empty
Expand Down Expand Up @@ -434,6 +447,19 @@ object ByteString {
}
}

override def indexOf(elem: Byte, from: Int): Int = {
if (from >= length) -1
else {
var found = -1
var i = math.max(from, 0)
while (i < length && found == -1) {
if (bytes(startIndex + i) == elem) found = i
i += 1
}
found
}
}

override def copyToArray[B >: Byte](dest: Array[B], start: Int, len: Int): Int = {
// min of the bytes available to copy, bytes there is room for in dest and the requested number of bytes
val toCopy = math.min(math.min(len, length), dest.length - start)
Expand Down Expand Up @@ -691,8 +717,33 @@ object ByteString {
} else {
val subIndexOf = bs.indexOf(elem, relativeIndex)
if (subIndexOf < 0) {
val nextString = bsIdx + 1
find(nextString, relativeIndex - bs.length, bytesPassed + bs.length)
find(bsIdx + 1, relativeIndex - bs.length, bytesPassed + bs.length)
} else subIndexOf + bytesPassed
}
}
}

find(0, math.max(from, 0), 0)
}
}

override def indexOf(elem: Byte, from: Int): Int = {
if (from >= length) -1
else {
val byteStringsSize = bytestrings.size

@tailrec
def find(bsIdx: Int, relativeIndex: Int, bytesPassed: Int): Int = {
if (bsIdx >= byteStringsSize) -1
else {
val bs = bytestrings(bsIdx)

if (bs.length <= relativeIndex) {
find(bsIdx + 1, relativeIndex - bs.length, bytesPassed + bs.length)
} else {
val subIndexOf = bs.indexOf(elem, relativeIndex)
if (subIndexOf < 0) {
find(bsIdx + 1, relativeIndex - bs.length, bytesPassed + bs.length)
} else subIndexOf + bytesPassed
}
}
Expand Down Expand Up @@ -821,8 +872,31 @@ sealed abstract class ByteString

override def indexWhere(p: Byte => Boolean, from: Int): Int = iterator.indexWhere(p, from)

// optimized in subclasses
override def indexOf[B >: Byte](elem: B, from: Int): Int = indexOf(elem, from)
// optimized version of indexOf for bytes, optimized in subclasses
/**
* Finds index of first occurrence of some byte in this ByteString after or at some start index.
*
* Similar to indexOf, but it avoids boxing if the value is already a byte.
*
* @param elem the element value to search for.
* @param from the start index
* @return the index `>= from` of the first element of this ByteString that is equal (as determined by `==`)
* to `elem`, or `-1`, if none exists.
* @since 1.1.0
*/
def indexOf(elem: Byte, from: Int): Int = indexOf[Byte](elem, from)

/**
* Finds index of first occurrence of some byte in this ByteString.
*
* Similar to indexOf, but it avoids boxing if the value is already a byte.
*
* @param elem the element value to search for.
* @return the index `>= from` of the first element of this ByteString that is equal (as determined by `==`)
* to `elem`, or `-1`, if none exists.
* @since 1.1.0
*/
def indexOf(elem: Byte): Int = indexOf(elem, 0)

override def grouped(size: Int): Iterator[ByteString] = {
if (size <= 0) {
Expand Down
Loading
Loading