Demonstration

.travis.yml

@@ -1,3 +1,7 @@
+# This is necessary until https://github.com/travis-ci/travis-ci/issues/3120 is
+# fixed
+sudo: required
+
 language: scala
 
 # These directories are cached to S3 at the end of the build
@@ -13,6 +17,10 @@ jdk:
   - oraclejdk7
   - openjdk7
 
-before_script: "unset SBT_OPTS"
+before_script:
+  - unset SBT_OPTS
+  - ./bin/travisci
+  - travis_retry ./sbt ++$TRAVIS_SCALA_VERSION update
 
-script: "./bin/travisci"
+script: ./sbt ++$TRAVIS_SCALA_VERSION coverage test && ./sbt ++$TRAVIS_SCALA_VERSION coverageAggregate
+after_success: ./sbt ++$TRAVIS_SCALA_VERSION coveralls

README.md

@@ -1,5 +1,8 @@
 # Scrooge
-[![Build Status](https://secure.travis-ci.org/twitter/scrooge.png?branch=master)](https://travis-ci.org/twitter/scrooge)
+
+[![Build status](https://travis-ci.org/travisbrown/scrooge.svg?branch=develop)](https://travis-ci.org/travisbrown/scrooge)
+[![Coverage status](https://img.shields.io/coveralls/travisbrown/scrooge/develop.svg)](https://coveralls.io/r/travisbrown/scrooge?branch=develop)
+[![Project status](https://img.shields.io/badge/status-active-brightgreen.svg)](#status)
 
 Scrooge is a [thrift](https://thrift.apache.org/) code generator written in
 Scala, which currently generates code for Scala and Java.
@@ -18,6 +21,11 @@ syntax so the generated code is much more compact.
 There is a fairly comprehensive set of unit tests, which actually generate
 code, compile it, and execute it to verify expectations.
 
+## Status
+
+This project is used in production at Twitter (and many other organizations),
+and is being actively developed and maintained.
+
 ## Quick-start
 
 There are a couple of classes needed by the generated code. These have been

bin/travisci

@@ -12,19 +12,20 @@ if [ "$SCROOGE_BRANCH" != "master" ]; then
   SCROOGE_TMP_DIR=$(mktemp -d -t scrooge.XXXXXXXXXX.tmp)
   # util
   cd $SCROOGE_TMP_DIR
-  git clone https://github.com/twitter/util.git --branch develop
+  git clone https://github.com/travisbrown/util.git --branch develop
   cd util
   $SCROOGE_SBT ++$TRAVIS_SCALA_VERSION publishLocal
   # ostrich
   cd $SCROOGE_TMP_DIR
-  git clone https://github.com/twitter/ostrich.git --branch develop
+  git clone https://github.com/travisbrown/ostrich.git --branch develop
   cd ostrich
   $SCROOGE_SBT ++$TRAVIS_SCALA_VERSION publishLocal
   # finagle
   cd $SCROOGE_TMP_DIR
-  git clone https://github.com/twitter/finagle.git --branch develop
+  git clone https://github.com/travisbrown/finagle.git --branch develop
   cd finagle
   $SCROOGE_SBT ++$TRAVIS_SCALA_VERSION finagle-core/publishLocal
+  $SCROOGE_SBT ++$TRAVIS_SCALA_VERSION finagle-httpx/publishLocal
   $SCROOGE_SBT ++$TRAVIS_SCALA_VERSION finagle-mux/publishLocal
   $SCROOGE_SBT ++$TRAVIS_SCALA_VERSION finagle-thrift/publishLocal
   $SCROOGE_SBT ++$TRAVIS_SCALA_VERSION finagle-thriftmux/publishLocal
@@ -33,5 +34,3 @@ if [ "$SCROOGE_BRANCH" != "master" ]; then
   cd $SCROOGE_DIR
   rm -rf $SCROOGE_TMP_DIR
 fi
-
-./sbt ++$TRAVIS_SCALA_VERSION test

project/Build.scala

@@ -9,6 +9,7 @@ import pl.project13.scala.sbt.JmhPlugin
 import sbtassembly.Plugin._
 import AssemblyKeys._
 import sbtbuildinfo.Plugin._
+import scoverage.ScoverageSbtPlugin
 
 object Scrooge extends Build {
   val branch = Process("git" :: "rev-parse" :: "--abbrev-ref" :: "HEAD" :: Nil).!!.trim
@@ -70,6 +71,13 @@ object Scrooge extends Build {
       "sonatype-public" at "https://oss.sonatype.org/content/groups/public"
     ),
 
+    ScoverageSbtPlugin.ScoverageKeys.coverageHighlighting := (
+      CrossVersion.partialVersion(scalaVersion.value) match {
+        case Some((2, 10)) => false
+        case _ => true
+      }
+    ),
+
     publishM2Configuration <<= (packagedArtifacts, checksums in publish, ivyLoggingLevel) map { (arts, cs, level) =>
       Classpaths.publishConfig(
         artifacts = arts,
@@ -175,6 +183,7 @@ object Scrooge extends Build {
       "com.github.scopt" %% "scopt" % "3.2.0",
       "com.novocode" % "junit-interface" % "0.8" % "test->default" exclude("org.mockito", "mockito-all"),
       "org.codehaus.plexus" % "plexus-utils" % "1.5.4",
+      "org.slf4j" % "slf4j-log4j12" % "1.6.6" % "test", // used in thrift transports
       "com.google.code.findbugs" % "jsr305" % "1.3.9",
       "commons-cli" % "commons-cli" % "1.2",
       finagle("core") exclude("org.mockito", "mockito-all"),

project/plugins.sbt

@@ -4,14 +4,13 @@ resolvers += Resolver.url(
 
 resolvers += "maven" at "http://repo1.maven.org/maven2/"
 
-addSbtPlugin("com.eed3si9n" % "sbt-assembly" % "0.11.2")
+resolvers += Classpaths.sbtPluginReleases
 
+addSbtPlugin("com.eed3si9n" % "sbt-assembly" % "0.11.2")
+addSbtPlugin("com.eed3si9n" % "sbt-buildinfo" % "0.3.2")
 addSbtPlugin("com.typesafe.sbt" % "sbt-site" % "0.6.2")
-
 addSbtPlugin("me.lessis" % "bintray-sbt" % "0.1.1")
-
 addSbtPlugin("net.virtual-void" % "sbt-dependency-graph" % "0.7.4")
-
-addSbtPlugin("com.eed3si9n" % "sbt-buildinfo" % "0.3.2")
-
-addSbtPlugin("pl.project13.scala" % "sbt-jmh" % "0.2.0")
+addSbtPlugin("org.scoverage" % "sbt-coveralls" % "1.0.0")
+addSbtPlugin("org.scoverage" % "sbt-scoverage" % "1.0.4")
+addSbtPlugin("pl.project13.scala" % "sbt-jmh" % "0.2.0")

scrooge-benchmark/src/main/scala/com/twitter/scrooge/benchmark/LazyTProtocol.scala

@@ -27,9 +27,7 @@ case class StatelessLazyBinaryProtocol[T <: ThriftStruct](codec: ThriftStructCod
 
 }
 
-// This is not thread safe but we store it in the thread context for the benchmarks
-// A realworld usage would probably want a thread local instance of the transport and protocol.
-case class StatefulLazyBinaryProtocol[T <: ThriftStruct](codec: ThriftStructCodec[T]) {
+case class ThreadUnsafeLazyBinaryProtocol[T <: ThriftStruct](codec: ThriftStructCodec[T]) {
   val transport = new TArrayByteTransport
   val proto = new TLazyBinaryProtocol(transport)
 
@@ -45,6 +43,7 @@ case class StatefulLazyBinaryProtocol[T <: ThriftStruct](codec: ThriftStructCode
   }
 }
 
+
 object LazyTProtocolBenchmark {
   // Pass in a seed and fixed number of airports.
   // Will be initialized with this object so separate from the benchmarks.
@@ -76,7 +75,7 @@ object LazyTProtocolBenchmark {
 
   @State(Scope.Thread)
   class AirportThreadState {
-    val statefulLazySerializer = StatefulLazyBinaryProtocol(Airport)
+    val threadUnsafeLazySerializer = ThreadUnsafeLazyBinaryProtocol(Airport)
   }
 
   @State(Scope.Benchmark)
@@ -86,14 +85,15 @@ object LazyTProtocolBenchmark {
 
     val statelessLazySerializer = StatelessLazyBinaryProtocol(Airport)
 
+    val statefulLazySerializer = LazyBinaryThriftStructSerializer(Airport)
+
     @Setup(Level.Trial)
     def setup(): Unit = {
-      require(airportBytes.forall{b => binaryThriftStructSerializer.fromBytes(b) == statelessLazySerializer.fromBytes(b)}, "Deserializers do not agree, benchmarks pointless")
-      require(airports.forall{b => ByteBuffer.wrap(binaryThriftStructSerializer.toBytes(b)) == ByteBuffer.wrap(statelessLazySerializer.toBytes(b))}, "Stateful vs normal Serializers do not agree, benchmarks pointless")
+      require(airportBytes.forall { b => binaryThriftStructSerializer.fromBytes(b) == statelessLazySerializer.fromBytes(b) } , "Deserializers do not agree, benchmarks pointless")
+      require(airports.forall { b => ByteBuffer.wrap(binaryThriftStructSerializer.toBytes(b)) == ByteBuffer.wrap(statelessLazySerializer.toBytes(b)) } , "Stateful vs normal Serializers do not agree, benchmarks pointless")
 
-      val statefulTmp = new AirportThreadState
-      require(airportBytes.forall{b => binaryThriftStructSerializer.fromBytes(b) == statefulTmp.statefulLazySerializer.fromBytes(b)}, "Stateful Deserializers do not agree, benchmarks pointless")
-      require(airports.forall{b => ByteBuffer.wrap(binaryThriftStructSerializer.toBytes(b)) == ByteBuffer.wrap(statefulTmp.statefulLazySerializer.toBytes(b))}, "Stateful Serializers do not agree, benchmarks pointless")
+      require(airportBytes.forall { b => binaryThriftStructSerializer.fromBytes(b) == statefulLazySerializer.fromBytes(b) } , "Stateful Deserializers do not agree, benchmarks pointless")
+      require(airports.forall { b => ByteBuffer.wrap(binaryThriftStructSerializer.toBytes(b)) == ByteBuffer.wrap(statefulLazySerializer.toBytes(b)) } , "Stateful Serializers do not agree, benchmarks pointless")
     }
 
   }
@@ -166,31 +166,61 @@ class LazyTProtocolBenchmark {
   // ========= Stateful benchmarks =========
 
   @Benchmark
-  def timeStatefulToBytes(state: AirportState, threadState: AirportThreadState): Seq[Array[Byte]] = {
-    airports.map(threadState.statefulLazySerializer.toBytes)
+  def timeStatefulToBytes(state: AirportState): Seq[Array[Byte]] = {
+    airports.map(state.statefulLazySerializer.toBytes)
+  }
+
+
+  @Benchmark
+  def timeStatefulFromBytes(state: AirportState): Seq[Airport] = {
+    airportBytes.map(state.statefulLazySerializer.fromBytes)
+  }
+
+  @Benchmark
+  def timeStatefulFromBytesRead3Fields(state: AirportState, bh: Blackhole): Blackhole = {
+    airportBytes.map(state.statefulLazySerializer.fromBytes).foreach(a => read3Fields(bh, a))
+    bh
+  }
+
+    @Benchmark
+  def timeStatefulFromBytesReadlAllFields(state: AirportState, bh: Blackhole): Blackhole = {
+    airportBytes.map(state.statefulLazySerializer.fromBytes).foreach(a => readAllFields(bh, a))
+    bh
+  }
+
+  @Benchmark
+  def timeStatefulRTBytes(state: AirportState): Seq[Array[Byte]] = {
+    airportBytes.map(b => state.statefulLazySerializer.toBytes(state.statefulLazySerializer.fromBytes(b)))
+  }
+
+ // ========= Using the thread state, no built in thread safety =========
+
+  @Benchmark
+  def timeThreadUnsafeToBytes(state: AirportState, threadState: AirportThreadState): Seq[Array[Byte]] = {
+    airports.map(threadState.threadUnsafeLazySerializer.toBytes)
   }
 
 
   @Benchmark
-  def timeStatefulFromBytes(state: AirportState, threadState: AirportThreadState): Seq[Airport] = {
-    airportBytes.map(threadState.statefulLazySerializer.fromBytes)
+  def timeThreadUnsafeFromBytes(state: AirportState, threadState: AirportThreadState): Seq[Airport] = {
+    airportBytes.map(threadState.threadUnsafeLazySerializer.fromBytes)
   }
 
   @Benchmark
-  def timeStatefulFromBytesRead3Fields(state: AirportState, threadState: AirportThreadState, bh: Blackhole): Blackhole = {
-    airportBytes.map(threadState.statefulLazySerializer.fromBytes).foreach(a => read3Fields(bh, a))
+  def timeThreadUnsafeFromBytesRead3Fields(state: AirportState, threadState: AirportThreadState, bh: Blackhole): Blackhole = {
+    airportBytes.map(threadState.threadUnsafeLazySerializer.fromBytes).foreach(a => read3Fields(bh, a))
     bh
   }
 
     @Benchmark
-  def timeStatefulFromBytesReadlAllFields(state: AirportState, threadState: AirportThreadState, bh: Blackhole): Blackhole = {
-    airportBytes.map(threadState.statefulLazySerializer.fromBytes).foreach(a => readAllFields(bh, a))
+  def timeThreadUnsafeFromBytesReadlAllFields(state: AirportState, threadState: AirportThreadState, bh: Blackhole): Blackhole = {
+    airportBytes.map(threadState.threadUnsafeLazySerializer.fromBytes).foreach(a => readAllFields(bh, a))
     bh
   }
 
   @Benchmark
-  def timeStatefulRTBytes(state: AirportState, threadState: AirportThreadState): Seq[Array[Byte]] = {
-    airportBytes.map(b => threadState.statefulLazySerializer.toBytes(threadState.statefulLazySerializer.fromBytes(b)))
+  def timeThreadUnsafeRTBytes(state: AirportState, threadState: AirportThreadState): Seq[Array[Byte]] = {
+    airportBytes.map(b => threadState.threadUnsafeLazySerializer.toBytes(threadState.threadUnsafeLazySerializer.fromBytes(b)))
   }
 
 }

scrooge-benchmark/src/main/scala/com/twitter/scrooge/benchmark/TArrayByteTransport.scala → scrooge-core/src/main/scala/com/twitter/scrooge/TArrayByteTransport.scala

@@ -1,30 +1,32 @@
-package com.twitter.scrooge.benchmark
+package com.twitter.scrooge
 
 import org.apache.thrift.protocol._
 import org.apache.thrift.TException
 import org.apache.thrift.transport.{TTransport, TTransportException}
 
 /**
  * TArrayByteTransport decodes Array[Byte] to primitive types
- * This is a proof of concept replacement transport optimized for Array Byte
+ * This is a replacement transport optimized for Array[Byte]
  * and the TLazyBinaryProtocol
- * It mixes reading and writing, in a real usage they possibly should be different
- * transports entirely to keep the code clean.
- * NB. This class/transport is not thread safe, or shareable.
+ *
+ * NB. This class/transport is not thread safe, and contains mutable state.
  */
 object TArrayByteTransport {
   def apply(buf: Array[Byte]): TArrayByteTransport = {
-    val t = new TArrayByteTransport
+    val t = new TArrayByteTransport(0) // No write buffer used in read path
     t.setBytes(buf)
     t
   }
 }
 
 final class TArrayByteTransport(initialWriteBufferSize: Int = 512) extends TTransport {
+  // Read state variables
   private[this] var bufferPos = 0
   private[this] var readbufferSiz_ = 0
   private[this] var srcBuf_ : Array[Byte] = null
 
+
+  // Write state variables
   private[this] var writeBuffers: List[(Array[Byte], Int)] = Nil
   private[this] var totalSize = 0
   private[this] var nextBufferSize = initialWriteBufferSize
@@ -86,6 +88,39 @@ final class TArrayByteTransport(initialWriteBufferSize: Int = 512) extends TTran
     }
   }
 
+
+  override def write(buf: Array[Byte], off: Int, len: Int): Unit = {
+    val dest = getBuffer(len)
+    val destOffset = writerOffset
+    System.arraycopy(buf, off, dest, destOffset, len)
+  }
+
+  /*
+   * Take our internal state and present it as a byte array
+   */
+  def toByteArray: Array[Byte] = {
+    (currentBuffer, writeBuffers) match {
+      case (null, Nil) => new Array[Byte](0)
+      case (buf, Nil) =>
+        val finalBuf = new Array[Byte](totalSize)
+        System.arraycopy(currentBuffer, 0, finalBuf, 0, totalSize)
+        finalBuf
+      case (buf, x) =>
+        var reverseOffset = totalSize - currentOffset
+        val finalBuf = new Array[Byte](totalSize)
+        System.arraycopy(currentBuffer, 0, finalBuf, reverseOffset, currentOffset)
+
+        writeBuffers.foreach {
+          case (buf, siz) =>
+            reverseOffset -= siz
+            System.arraycopy(buf, 0, finalBuf, reverseOffset, siz)
+        }
+        finalBuf
+    }
+  }
+
+  // Read methods from here:
+
   // Only used in reading to give a pointer to the Array[Byte]
   // backing this.
   // Should only be used very carefully, since its mutable
@@ -121,36 +156,6 @@ final class TArrayByteTransport(initialWriteBufferSize: Int = 512) extends TTran
   override def getBytesRemainingInBuffer: Int = readbufferSiz_ - bufferPos
   override def getBuffer: Array[Byte] = srcBuf_
 
-  override def write(buf: Array[Byte], off: Int, len: Int): Unit = {
-    val dest = getBuffer(len)
-    val destOffset = writerOffset
-    System.arraycopy(buf, off, dest, destOffset, len)
-  }
-
-  /*
-   * Take our internal state and present it as a byte array
-   */
-  def toByteArray: Array[Byte] = {
-    (currentBuffer, writeBuffers) match {
-      case (null, Nil) => new Array[Byte](0)
-      case (buf, Nil) =>
-        val finalBuf = new Array[Byte](totalSize)
-        System.arraycopy(currentBuffer, 0, finalBuf, 0, totalSize)
-        finalBuf
-      case (buf, x) =>
-        var reverseOffset = totalSize - currentOffset
-        val finalBuf = new Array[Byte](totalSize)
-        System.arraycopy(currentBuffer, 0, finalBuf, reverseOffset, currentOffset)
-
-        writeBuffers.foreach {
-          case (buf, siz) =>
-            reverseOffset -= siz
-            System.arraycopy(buf, 0, finalBuf, reverseOffset, siz)
-        }
-        finalBuf
-    }
-  }
-
   override def consumeBuffer(len: Int): Unit = {
     bufferPos = bufferPos + len
   }

scrooge-benchmark/src/main/scala/com/twitter/scrooge/benchmark/TLazyBinaryProtocol.scala → scrooge-core/src/main/scala/com/twitter/scrooge/TLazyBinaryProtocol.scala

@@ -1,11 +1,9 @@
-package com.twitter.scrooge.benchmark
+package com.twitter.scrooge
 
-import com.twitter.scrooge.LazyTProtocol
 import java.io.UnsupportedEncodingException
 import java.nio.ByteBuffer
 import org.apache.thrift.protocol._
 import org.apache.thrift.TException
-import org.apache.thrift.transport.TTransport
 
 /**
  * This is an implementation of the LazyTProtocol trait in scrooge-core
@@ -15,7 +13,7 @@ import org.apache.thrift.transport.TTransport
  * when it comes to a version for scrooge-serializer or elsewhere.
  * Though it is a fully functional protocol that will deserialize/serialize any thrift.
  */
-private object TLazyBinaryProtocol {
+object TLazyBinaryProtocol {
   private val AnonymousStruct: TStruct = new TStruct()
 }