Skip to content

Commit

Permalink
util/finagle: Small changes to avoid allocations
Browse files Browse the repository at this point in the history
Problem

In various places we use methods that rely on Scala's implicits to
enrich them. This typically at the cost of an unnecessary object
allocation. There are some other simple changes to be made that
avoid allocations as well.

Solution

Use methods that do not require allocations. Some examples:

* String.nonEmpty => !String.isEmpty
* String.size => String.length
* StringBuilder.size => StringBuilder.length
* Array.size => Array.length
* Seq.empty => Nil

RB_ID=630964
  • Loading branch information
kevinoliver authored and jenkins committed Apr 13, 2015
1 parent d4595fe commit 09d490f
Show file tree
Hide file tree
Showing 25 changed files with 126 additions and 115 deletions.
Expand Up @@ -25,7 +25,7 @@ class SourceTrackingMonitor(logger: Logger, which: String) extends Monitor {
false
}

private[this] def unrollCauses(exc: Throwable, res: Seq[String] = Seq()): Seq[String] = exc match {
private[this] def unrollCauses(exc: Throwable, res: Seq[String] = Nil): Seq[String] = exc match {
case null => res.reverse
case se: SourcedException => unrollCauses(se.getCause, se.serviceName +: res)
case fail: Failure => fail.getSource(Failure.Source.Service) match {
Expand Down
Expand Up @@ -86,7 +86,7 @@ class SocksConnectHandler(
private[this] def writeInit(ctx: ChannelHandlerContext) {
val buf = ChannelBuffers.dynamicBuffer(1024)
buf.writeByte(Version5)
buf.writeByte(supportedTypes.size.toByte)
buf.writeByte(supportedTypes.length.toByte)
buf.writeBytes(supportedTypes)

write(ctx, buf)
Expand Down Expand Up @@ -117,7 +117,7 @@ class SocksConnectHandler(
case _ => // unresolved host
buf.writeByte(HostnameIndicator)
val hostnameBytes = addr.getHostName.getBytes(Charsets.UsAscii)
buf.writeByte(hostnameBytes.size)
buf.writeByte(hostnameBytes.length)
buf.writeBytes(hostnameBytes)
}

Expand All @@ -132,11 +132,11 @@ class SocksConnectHandler(

// RFC does not specify an encoding. Assume UTF8
val usernameBytes = username.getBytes(Charsets.Utf8)
buf.writeByte(usernameBytes.size.toByte)
buf.writeByte(usernameBytes.length.toByte)
buf.writeBytes(usernameBytes)

val passBytes = pass.getBytes(Charsets.Utf8)
buf.writeByte(passBytes.size.toByte)
buf.writeByte(passBytes.length.toByte)
buf.writeBytes(passBytes)

write(ctx, buf)
Expand Down
Expand Up @@ -8,7 +8,7 @@ package com.twitter.finagle.stats
import com.google.common.util.concurrent.AtomicLongMap
import com.google.common.cache.{CacheBuilder, CacheLoader}
import scala.collection.JavaConverters._
import scala.collection.mutable.{ArrayBuffer, SynchronizedBuffer}
import scala.collection.mutable.ArrayBuffer

class SummarizingStatsReceiver extends StatsReceiverWithCumulativeGauges {
val repr = this
Expand All @@ -30,7 +30,7 @@ class SummarizingStatsReceiver extends StatsReceiverWithCumulativeGauges {
}

def stat(name: String*) = new Stat {
def add(value: Float) = SummarizingStatsReceiver.this.synchronized {
def add(value: Float) = SummarizingStatsReceiver.this.synchronized {
stats.get(name) += value
}
}
Expand Down Expand Up @@ -67,14 +67,14 @@ class SummarizingStatsReceiver extends StatsReceiverWithCumulativeGauges {

val counterLines = (counterValues map { case (k, v) => (variableName(k), v.toString) }).toSeq
val statLines = (statValues map { case (k, xs) =>
val n = xs.size
val n = xs.length
def idx(ptile: Double) = math.floor(ptile*n).toInt
(variableName(k), "n=%d min=%.1f med=%.1f p90=%.1f p95=%.1f p99=%.1f p999=%.1f p9999=%.1f max=%.1f".format(
n, xs(0), xs(n/2), xs(idx(.9D)), xs(idx(.95D)), xs(idx(.99D)), xs(idx(.999D)), xs(idx(.9999D)), xs(n-1)))
}).toSeq

lazy val tailValues = (statValues map { case (k, xs) =>
val n = xs.size
val n = xs.length
def slice(ptile: Double) = {
val end = math.floor(ptile*n).toInt
val start = math.ceil(end-((1.0-ptile)*n)).toInt
Expand Down
Expand Up @@ -38,7 +38,7 @@ object InetSocketAddressUtil {
*/
def parseHostPorts(hosts: String): Seq[HostPort] =
hosts split Array(' ', ',') filter (_.nonEmpty) map (_.split(":")) map { hp =>
require(hp.size == 2, "You must specify host and port")
require(hp.length == 2, "You must specify host and port")
hp match {
case Array(host, "*") => (host, 0)
case Array(host, portStr) => (host, portStr.toInt)
Expand Down
Expand Up @@ -147,10 +147,10 @@ private object ClassPath {
val commentIdx = line.indexOf('#')
val end = if (commentIdx != -1) commentIdx else line.length
val str = line.substring(0, end).trim
if (str.isEmpty) Seq.empty else Seq(str)
if (str.isEmpty) Nil else Seq(str)
}
} catch {
case ex: MalformedInputException => Seq.empty /* skip malformed files (e.g. non UTF-8) */
case ex: MalformedInputException => Nil /* skip malformed files (e.g. non UTF-8) */
} finally {
source.close()
}
Expand Down
Expand Up @@ -69,10 +69,10 @@ object HttpDtab {
}

private def validHeaderPair(aKey: String, bKey: String): Boolean =
aKey.size == bKey.size &&
aKey.substring(0, aKey.size-1) == bKey.substring(0, bKey.size-1) &&
aKey(aKey.size-1) == 'a' &&
bKey(bKey.size-1) == 'b'
aKey.length == bKey.length &&
aKey.charAt(aKey.length - 1) == 'a' &&
bKey.charAt(bKey.length - 1) == 'b' &&
aKey.substring(0, aKey.length - 1) == bKey.substring(0, bKey.length - 1)

private val EmptyReturn = Return(Dtab.empty)

Expand All @@ -99,7 +99,7 @@ object HttpDtab {
// TODO: now that we have a proper Dtab grammar,
// should just embed this directly instead.
msg.headers.set(Prefix+indexstr(i)+"-A", b64Encode(prefix.show))
msg.headers.set(Prefix+indexstr(i)+"-B".format(i), b64Encode(dst.show))
msg.headers.set(Prefix+indexstr(i)+"-B", b64Encode(dst.show))
}
}

Expand Down
Expand Up @@ -37,7 +37,7 @@ object Cors {
allowsOrigin: String => Option[String],
allowsMethods: String => Option[Seq[String]],
allowsHeaders: Seq[String] => Option[Seq[String]],
exposedHeaders: Seq[String] = Seq.empty,
exposedHeaders: Seq[String] = Nil,
supportsCredentials: Boolean = false,
maxAge: Option[Duration] = None)

Expand Down
Expand Up @@ -69,10 +69,10 @@ object HttpDtab {
}

private def validHeaderPair(aKey: String, bKey: String): Boolean =
aKey.size == bKey.size &&
aKey.substring(0, aKey.size-1) == bKey.substring(0, bKey.size-1) &&
aKey(aKey.size-1) == 'a' &&
bKey(bKey.size-1) == 'b'
aKey.length == bKey.length &&
aKey(aKey.length - 1) == 'a' &&
bKey(bKey.length - 1) == 'b' &&
aKey.substring(0, aKey.length - 1) == bKey.substring(0, bKey.length - 1)

private val EmptyReturn = Return(Dtab.empty)

Expand All @@ -99,7 +99,7 @@ object HttpDtab {
// TODO: now that we have a proper Dtab grammar,
// should just embed this directly instead.
msg.headers.set(Prefix+indexstr(i)+"-A", b64Encode(prefix.show))
msg.headers.set(Prefix+indexstr(i)+"-B".format(i), b64Encode(dst.show))
msg.headers.set(Prefix+indexstr(i)+"-B", b64Encode(dst.show))
}
}

Expand Down
Expand Up @@ -36,7 +36,7 @@ object Cors {
allowsOrigin: String => Option[String],
allowsMethods: String => Option[Seq[String]],
allowsHeaders: Seq[String] => Option[Seq[String]],
exposedHeaders: Seq[String] = Seq.empty,
exposedHeaders: Seq[String] = Nil,
supportsCredentials: Boolean = false,
maxAge: Option[Duration] = None)

Expand Down
Expand Up @@ -70,7 +70,7 @@ class Decoder extends AbstractDecoder with StateMachine {
final protected[memcached] def awaitData(tokens: Seq[ChannelBuffer], bytesNeeded: Int) = {
state match {
case AwaitingResponse() =>
awaitData(Seq(), tokens, bytesNeeded)
awaitData(Nil, tokens, bytesNeeded)
case AwaitingResponseOrEnd(valuesSoFar) =>
awaitData(valuesSoFar, tokens, bytesNeeded)
}
Expand Down
Expand Up @@ -42,7 +42,7 @@ private[finagle] object ChannelBufferUtils {
split(FIND_SPACE, 1)

def split(delimiter: String): Seq[ChannelBuffer] =
split(stringToChannelBufferIndexFinder(delimiter), delimiter.size)
split(stringToChannelBufferIndexFinder(delimiter), delimiter.length)

def split(indexFinder: ChannelBufferIndexFinder, delimiterLength: Int): Seq[ChannelBuffer] = {
val tokens = new ArrayBuffer[ChannelBuffer]
Expand Down Expand Up @@ -101,14 +101,13 @@ private[finagle] object ChannelBufferUtils {
implicit def stringToChannelBufferIndexFinder(string: String): ChannelBufferIndexFinder =
new ChannelBufferIndexFinder {
def find(buffer: ChannelBuffer, guessedIndex: Int): Boolean = {
val array = string.toArray
var i: Int = 0
while (i < string.size) {
if (buffer.getByte(guessedIndex + i) != array(i).toByte)
var i = 0
while (i < string.length) {
if (buffer.getByte(guessedIndex + i) != string.charAt(i).toByte)
return false
i += 1
}
return true
true
}
}
}
Expand Up @@ -72,7 +72,7 @@ class Decoder extends AbstractDecoder with StateMachine {
final protected[memcachedx] def awaitData(tokens: Seq[ChannelBuffer], bytesNeeded: Int) = {
state match {
case AwaitingResponse() =>
awaitData(Seq(), tokens, bytesNeeded)
awaitData(Nil, tokens, bytesNeeded)
case AwaitingResponseOrEnd(valuesSoFar) =>
awaitData(valuesSoFar, tokens, bytesNeeded)
}
Expand Down
Expand Up @@ -42,7 +42,7 @@ private[finagle] object ChannelBufferUtils {
split(FIND_SPACE, 1)

def split(delimiter: String): Seq[ChannelBuffer] =
split(stringToChannelBufferIndexFinder(delimiter), delimiter.size)
split(stringToChannelBufferIndexFinder(delimiter), delimiter.length)

def split(indexFinder: ChannelBufferIndexFinder, delimiterLength: Int): Seq[ChannelBuffer] = {
val tokens = new ArrayBuffer[ChannelBuffer]
Expand Down Expand Up @@ -101,14 +101,13 @@ private[finagle] object ChannelBufferUtils {
implicit def stringToChannelBufferIndexFinder(string: String): ChannelBufferIndexFinder =
new ChannelBufferIndexFinder {
def find(buffer: ChannelBuffer, guessedIndex: Int): Boolean = {
val array = string.toArray
var i: Int = 0
while (i < string.size) {
if (buffer.getByte(guessedIndex + i) != array(i).toByte)
var i = 0
while (i < string.length) {
if (buffer.getByte(guessedIndex + i) != string.charAt(i).toByte)
return false
i += 1
}
return true
true
}
}
}
20 changes: 11 additions & 9 deletions finagle-mux/src/main/scala/com/twitter/finagle/mux/Proto.scala
Expand Up @@ -193,7 +193,7 @@ private[finagle] object Message {
}

val dstbytes = if (dst.isEmpty) noBytes else dst.show.getBytes(Charsets.Utf8)
n += 2 + dstbytes.size
n += 2 + dstbytes.length
n += 2

val dtabbytes = new Array[(Array[Byte], Array[Byte])](dtab.size)
Expand All @@ -204,7 +204,7 @@ private[finagle] object Message {
val srcbytes = src.show.getBytes(Charsets.Utf8)
val treebytes = tree.show.getBytes(Charsets.Utf8)

n += srcbytes.size + 2 + treebytes.size + 2
n += srcbytes.length + 2 + treebytes.length + 2

dtabbytes(dtabidx) = (srcbytes, treebytes)
dtabidx += 1
Expand All @@ -225,16 +225,16 @@ private[finagle] object Message {
seq = seq.tail
}

hd.writeShort(dstbytes.size)
hd.writeShort(dstbytes.length)
hd.writeBytes(dstbytes)

hd.writeShort(dtab.size)
dtabidx = 0
while (dtabidx != dtabbytes.size) {
while (dtabidx != dtabbytes.length) {
val (srcbytes, treebytes) = dtabbytes(dtabidx)
hd.writeShort(srcbytes.size)
hd.writeShort(srcbytes.length)
hd.writeBytes(srcbytes)
hd.writeShort(treebytes.size)
hd.writeShort(treebytes.length)
hd.writeBytes(treebytes)
dtabidx += 1
}
Expand Down Expand Up @@ -439,8 +439,10 @@ private[finagle] object Message {
nkeys -= 1
}

val id = trace3 map { case (spanId, parentId, traceId) =>
TraceId(Some(traceId), Some(parentId), spanId, None, Flags(traceFlags))
val id = trace3 match {
case Some((spanId, parentId, traceId)) =>
Some(TraceId(Some(traceId), Some(parentId), spanId, None, Flags(traceFlags)))
case None => None
}

Treq(tag, id, buf.slice())
Expand All @@ -449,7 +451,7 @@ private[finagle] object Message {
private def decodeContexts(buf: ChannelBuffer): Seq[(ChannelBuffer, ChannelBuffer)] = {
val n = buf.readUnsignedShort()
if (n == 0)
return Seq.empty
return Nil

val contexts = new Array[(ChannelBuffer, ChannelBuffer)](n)
var i = 0
Expand Down

0 comments on commit 09d490f

Please sign in to comment.