scrooge-serializer: Cache maxReusableBufferSize value in ThriftStruct…

…Serializer instances

Problem

Profiling serialization-heavy workflows shows `ThriftStructSerializer`
continuously retrieving the value of `maxReusableBufferSize` flag.

Solution

Cache the value of the `maxReusableBufferSize` on `ThriftStructSerializer`
instance creation, treat it as fixed during the instance lifetime. The value of
this flag typically does not change during the run time of an application, so
this was deemed an acceptable tradeoff.

JIRA Issues: CSL-6421

Differential Revision: https://phabricator.twitter.biz/D783669

CHANGELOG.rst

@@ -7,6 +7,15 @@ Note that ``PHAB_ID=#`` and ``RB_ID=#`` correspond to associated messages in com
 Unreleased
 ----------
 
+Runtime Behavior Changes
+~~~~~~~~~~~~~~~~~~~~~~~~
+
+* scrooge-serializer: concrete implementations of the `ThriftStructSerializer`
+  trait in the `c.t.scrooge.` package now cache the value of its `maxReusableBufferSize`
+  flag for the duration of the application. This improves performance but also makes them
+  not observe changes to the flag. The value of this flag typically does not change during
+  run time of an application, so this is deemed an acceptable tradeoff. ``PHAB_ID=`D783669`
+
 21.10.0
 -------
 

scrooge-serializer/src/main/scala/com/twitter/scrooge/ThriftStructSerializer.scala

@@ -1,8 +1,10 @@
 package com.twitter.scrooge
 
 import com.twitter.app.GlobalFlag
-import com.twitter.util.{Base64StringEncoder, StringEncoder}
-import java.io.{ByteArrayInputStream, InputStream}
+import com.twitter.util.Base64StringEncoder
+import com.twitter.util.StringEncoder
+import java.io.ByteArrayInputStream
+import java.io.InputStream
 import java.util.concurrent.atomic.AtomicLong
 import org.apache.thrift.protocol._
 import org.apache.thrift.transport.TIOStreamTransport
@@ -19,12 +21,13 @@ private object ThriftStructSerializer {
 
   val transportTooBig: AtomicLong = new AtomicLong(0)
 
+  private val maxRBS = maxReusableBufferSize()
+
   val reusableTransport: ThreadLocal[TReusableMemoryTransport] =
     new ThreadLocal[TReusableMemoryTransport] {
       override def initialValue(): TReusableMemoryTransport =
-        TReusableMemoryTransport(maxReusableBufferSize())
+        TReusableMemoryTransport(maxRBS)
     }
-
 }
 
 trait ThriftStructSerializer[T <: ThriftStruct] {
@@ -43,7 +46,7 @@ trait ThriftStructSerializer[T <: ThriftStruct] {
       trans.read(bytes, 0, trans.length())
       bytes
     } finally {
-      if (trans.currentCapacity > maxReusableBufferSize()) {
+      if (trans.currentCapacity > maxRBS) {
         transportTooBig.incrementAndGet()
         reusableTransport.remove()
       } else {

scrooge-serializer/src/test/scala/com/twitter/scrooge/ThriftStructSerializerTest.scala

@@ -1,5 +1,6 @@
 package com.twitter.scrooge
 
+import com.twitter.scrooge.serializer.thriftscala.SerializerStringTest
 import com.twitter.scrooge.serializer.thriftscala.SerializerTest
 import org.junit.runner.RunWith
 import org.scalatest.funsuite.AnyFunSuite
@@ -20,13 +21,11 @@ class ThriftStructSerializerTest extends AnyFunSuite {
 
   test("transportTooBig counter") {
     val startCount = ThriftStructSerializer.transportTooBig.get()
-    val instance = SerializerTest(5)
-    maxReusableBufferSize.let(0) {
-      val tss = BinaryThriftStructSerializer(SerializerTest)
-      val bytes = tss.toBytes(instance)
-      val andBack = tss.fromBytes(bytes)
-      assert(instance == andBack)
-    }
+    val instance = SerializerStringTest("*" * maxReusableBufferSize() + 1)
+    val tss = BinaryThriftStructSerializer(SerializerStringTest)
+    val bytes = tss.toBytes(instance)
+    val andBack = tss.fromBytes(bytes)
+    assert(instance == andBack)
     assert(ThriftStructSerializer.transportTooBig.get() == startCount + 1)
   }