はじめに
こんにちは、計測プラットフォーム部バックエンドチームのリーダー、児島（@cozima0210）です。この記事では、今年4月に社内で策定されたOSSポリシーに基づいて、チームでOSSに貢献する活動に取り組んだ話を紹介します。社内のOSSポリシーが策定された経緯については、こちらの記事をご覧ください。
なお、これはZOZOテクノロジーズ Advent Calender 2020 #3の5日目の記事です。
背景
私たちのチームでは、ZOZOSUIT/ZOZOMATから生成されるデータ及びそれを元とする計算データを高速に扱うため、様々なライブラリの使用を試みてきました。それらの中には、調査や試用の段階で不具合を発見したライブラリがありました。しかし、プロダクトの開発及び運用の過程では、そうした不具合の根本原因を探る時間を持つことは難しいものでした。そのため、代替ライブラリの選択を検討したり、使用する側でワークアラウンドの検討をすることにより、その問題を回避する状況が続いていました。
もちろん、その不具合がIssuesとして既出であるかの確認はしていました。ただ、それが認知されていない問題であったとしても、コミュニティに対する関わり方としてはそれ以上のことは何もできていませんでした。というのは、OSSに対する貢献が趣味の範囲であり、業務時間を使ってやるものではないという暗黙の了解があるように感じていたためです。しかし、こうした状況に対し、今年の4月に弊社ではOSSポリシーが策定されました。これにより、会社全体にOSSへの貢献に対するポジティブな機運の高まりを感じられるようになりました。
OSSにPRを送る会
チームの事情として、社内にOSSポリシーが策定された4月頃は、2月にZOZOMATをローンチした直後の時期でした。そして、ZOZOMATのアプリケーションが安定的に稼働するようになり、次のプロダクト開発に備える時期と重なっていました。そこで、これまでに累積的に認知していたライブラリの不具合に対し、チームとして取り組む活動をすることにしました。
具体的には、週に2時間チームで集まって、その不具合の調査をする時間を持つようにしました。まず、その不具合を初めて確認した時点からは時間も経過していたため、依然その不具合が発生する状況かどうかの確認をしました。そのため、改めてOSSのIssuesを網羅的にチェックし、不具合の再現に取り組みました。その不具合が再現可能であることを確認した上で、不具合を発生させている箇所を特定するためのコードリーディングをチームで行いました。そして、その問題が自分たちで修正可能であればPull Request（以降、PR）を送る、それが難しい場合でも少なくともIssueを立てることを目指しました。
こうした活動の中から、実際にIssueを送り、最終的にそのOSSでPRが作成されマージされるまでに至った事例を紹介します。
MessagePackを扱うためのライブラリ
ZOZOSUITの開発を始める際、大容量な3Dメッシュデータをサーバーで扱わなければいけない要件がありました。これを扱うにあたり、JSONで扱うにはサイズが大きくなりすぎるため、これをいかに小さく扱うことができるかを調べていました。その時に、シリアライズ及びデシリアライズする方式として、MessagePackの調査をしました。私たちのチームでは、バックエンドの開発言語にScalaを採用しており、候補としてはmsgpack-javaとmsgpack4zの2つがありました。そして、Scalaでのライブラリの選定を行う場合、jmhを使用し、簡易的なベンチマークを取得していました。そのため、以下のようなサンプルクラスのシリアライズとデシリアライズのパフォーマンス比較を行いました。
package jmh case class Mesh( faces: Seq[(Int, Int, Int)], vertices: Seq[(BigDecimal, BigDecimal, BigDecimal)] )
ライブラリの比較と見つかった不具合
まず、以下のコードでシリアライズのパフォーマンスを比較しました。今回の記事用に、Meshのデータサイズを実際よりも小さくしていますが、facesの要素数は約8,000、verticesの要素数は約4,000です。
build.sbt
... scalaVersion := "2.13.4", libraryDependencies ++= Seq( "io.circe" %% "circe-generic" % "0.13.0", "com.fasterxml.jackson.module" %% "jackson-module-scala" % "2.12.0", "org.msgpack" % "jackson-dataformat-msgpack" % "0.8.20", "com.github.xuwei-k" %% "msgpack4z-circe" % "0.12.0", "com.github.xuwei-k" %% "msgpack4z-native" % "0.3.6" ) ...
MessagePackSerializerBenchmark.scala
package jmh import java.util.concurrent.TimeUnit import com.fasterxml.jackson.databind.ObjectMapper import com.fasterxml.jackson.module.scala.DefaultScalaModule import com.fasterxml.jackson.module.scala.experimental.ScalaObjectMapper import io.circe.generic.auto._ import io.circe.syntax._ import msgpack4z.{CirceMsgpack, CirceUnpackOptions, MsgOutBuffer} import org.msgpack.jackson.dataformat.MessagePackFactory import org.openjdk.jmh.annotations.{Benchmark, BenchmarkMode, Mode, OutputTimeUnit, Scope, State} @State(Scope.Thread) class MessagePackSerializerBenchmark { val mapper = new ObjectMapper(new MessagePackFactory) with ScalaObjectMapper mapper.registerModule(DefaultScalaModule) val codec = CirceMsgpack.jsonCodec(CirceUnpackOptions.default) val mesh = Mesh( Seq( (1, 2, 3), (4, 5, 6), (7, 8, 9) ), Seq( (BigDecimal(1.11111), BigDecimal(2.22222), BigDecimal(3.33333)), (BigDecimal(4.44444), BigDecimal(5.55555), BigDecimal(6.66666)), (BigDecimal(7.77777), BigDecimal(8.88888), BigDecimal(9.99999)) ) ) @Benchmark @BenchmarkMode(Array(Mode.Throughput, Mode.AverageTime)) @OutputTimeUnit(TimeUnit.MILLISECONDS) def byJackson(): Array[Byte] = mapper.writeValueAsBytes(mesh) @Benchmark @BenchmarkMode(Array(Mode.Throughput, Mode.AverageTime)) @OutputTimeUnit(TimeUnit.MILLISECONDS) def byMsgpack4z(): Array[Byte] = { val packer = MsgOutBuffer.create() codec.toBytes(mesh.asJson, packer) } }
結果は、以下のようになりました。
> jmh/jmh:run -i 20 -wi 20 -f1 .MessagePackSerializerBenchmark. ... [info] Benchmark Mode Cnt Score Error Units [info] MessagePackSerializerBenchmark.byJackson thrpt 20 94.949 ± 4.638 ops/ms [info] MessagePackSerializerBenchmark.byMsgpack4z thrpt 20 167.041 ± 35.359 ops/ms [info] MessagePackSerializerBenchmark.byJackson avgt 20 0.017 ± 0.007 ms/op [info] MessagePackSerializerBenchmark.byMsgpack4z avgt 20 0.006 ± 0.001 ms/op
この結果から、シリアライズにおいてはmsgpack4zのスコアが優れていることがわかりました。
続いて、以下のコードでデシリアライズのパフォーマンスを比較しました。
MessagePackDeserializerBenchmark.scala
package jmh
import java.util.concurrent.TimeUnit
import com.fasterxml.jackson.databind.ObjectMapper
import com.fasterxml.jackson.module.scala.DefaultScalaModule
import com.fasterxml.jackson.module.scala.experimental.ScalaObjectMapper
import io.circe.generic.auto._
import io.circe.syntax._
import msgpack4z.{CirceMsgpack, CirceUnpackOptions, MsgInBuffer, MsgOutBuffer}
import org.msgpack.jackson.dataformat.MessagePackFactory
import org.openjdk.jmh.annotations.{Benchmark, BenchmarkMode, Mode, OutputTimeUnit, Scope, State}
@State(Scope.Thread)
class MessagePackDeserializerBenchmark {
val mapper = new ObjectMapper(new MessagePackFactory) with ScalaObjectMapper
mapper.registerModule(DefaultScalaModule)
val codec = CirceMsgpack.jsonCodec(CirceUnpackOptions.default)
val mesh = Mesh(
Seq(
(1, 2, 3),
(4, 5, 6),
(7, 8, 9)
),
Seq(
(BigDecimal(1.11111), BigDecimal(2.22222), BigDecimal(3.33333)),
(BigDecimal(4.44444), BigDecimal(5.55555), BigDecimal(6.66666)),
(BigDecimal(7.77777), BigDecimal(8.88888), BigDecimal(9.99999))
)
)
val bytes: Array[Byte] = codec.toBytes(mesh.asJson, MsgOutBuffer.create())
@Benchmark
@BenchmarkMode(Array(Mode.Throughput, Mode.AverageTime))
@OutputTimeUnit(TimeUnit.MILLISECONDS)
def byJackson(): Unit = mapper.readValue[Mesh](bytes)
@Benchmark
@BenchmarkMode(Array(Mode.Throughput, Mode.AverageTime))
@OutputTimeUnit(TimeUnit.MILLISECONDS)
def byMsgpack4z(): Unit = {
val unpacker = MsgInBuffer(bytes)
codec.unpack(unpacker)
}
}
しかし、以下のようなログが出力されて、デシリアライズのパフォーマンスは比較できませんでした。
> jmh/jmh:run -i 20 -wi 20 -f1 .MessagePackDeserializerBenchmark. ... [info] # Run progress: 0.00% complete, ETA 00:08:00 [info] # Fork: 1 of 3 [info] # Warmup Iteration 1: <failure> [info] com.fasterxml.jackson.databind.JsonMappingException: Invalid type=DOUBLE (through reference chain: jmh.Mesh["vertices"]->com.fasterxml.jackson.module.scala.deser.GenericFactoryDeserializerResolver$BuilderWrapper[0]) [info] at com.fasterxml.jackson.databind.JsonMappingException.wrapWithPath(JsonMappingException.java:390) [info] at com.fasterxml.jackson.databind.JsonMappingException.wrapWithPath(JsonMappingException.java:361) [info] at com.fasterxml.jackson.databind.deser.std.CollectionDeserializer._deserializeFromArray(CollectionDeserializer.java:363) [info] at com.fasterxml.jackson.databind.deser.std.CollectionDeserializer.deserialize(CollectionDeserializer.java:244) [info] at com.fasterxml.jackson.module.scala.deser.GenericFactoryDeserializerResolver$Deserializer.deserialize(GenericFactoryDeserializerResolver.scala:82) [info] at com.fasterxml.jackson.databind.deser.SettableBeanProperty.deserialize(SettableBeanProperty.java:542) [info] at com.fasterxml.jackson.databind.deser.BeanDeserializer._deserializeWithErrorWrapping(BeanDeserializer.java:565) [info] at com.fasterxml.jackson.databind.deser.BeanDeserializer._deserializeUsingPropertyBased(BeanDeserializer.java:449) [info] at com.fasterxml.jackson.databind.deser.BeanDeserializerBase.deserializeFromObjectUsingNonDefault(BeanDeserializerBase.java:1390) [info] at com.fasterxml.jackson.databind.deser.BeanDeserializer.deserializeFromObject(BeanDeserializer.java:362) [info] at com.fasterxml.jackson.databind.deser.BeanDeserializer.deserialize(BeanDeserializer.java:195) [info] at com.fasterxml.jackson.databind.deser.DefaultDeserializationContext.readRootValue(DefaultDeserializationContext.java:322) [info] at com.fasterxml.jackson.databind.ObjectMapper._readMapAndClose(ObjectMapper.java:4591) [info] at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:3641) [info] at com.fasterxml.jackson.module.scala.ScalaObjectMapper.readValue(ScalaObjectMapper.scala:203) [info] at com.fasterxml.jackson.module.scala.ScalaObjectMapper.readValue$(ScalaObjectMapper.scala:202) [info] at jmh.MessagePackDeserializerBenchmark$$anon$1.readValue(MessagePackDeserializerBenchmark.scala:17) [info] at jmh.MessagePackDeserializerBenchmark.byJackson(MessagePackDeserializerBenchmark.scala:40) [info] at jmh.generated.MessagePackDeserializerBenchmark_byJackson_jmhTest.byJackson_thrpt_jmhStub(MessagePackDeserializerBenchmark_byJackson_jmhTest.java:119) [info] at jmh.generated.MessagePackDeserializerBenchmark_byJackson_jmhTest.byJackson_Throughput(MessagePackDeserializerBenchmark_byJackson_jmhTest.java:83) [info] at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) [info] at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) [info] at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) [info] at java.lang.reflect.Method.invoke(Method.java:498) [info] at org.openjdk.jmh.runner.BenchmarkHandler$BenchmarkTask.call(BenchmarkHandler.java:453) [info] at org.openjdk.jmh.runner.BenchmarkHandler$BenchmarkTask.call(BenchmarkHandler.java:437) [info] at java.util.concurrent.FutureTask.run(FutureTask.java:266) [info] at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511) [info] at java.util.concurrent.FutureTask.run(FutureTask.java:266) [info] at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149) [info] at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624) [info] at java.lang.Thread.run(Thread.java:823) [info] Caused by: java.lang.IllegalStateException: Invalid type=DOUBLE [info] at org.msgpack.jackson.dataformat.MessagePackParser.getText(MessagePackParser.java:387) [info] at com.fasterxml.jackson.module.scala.deser.BigNumberDeserializer.deserialize(ScalaNumberDeserializersModule.scala:20) [info] at com.fasterxml.jackson.module.scala.deser.TupleDeserializer.$anonfun$deserialize$1(TupleDeserializerModule.scala:48) [info] at com.fasterxml.jackson.module.scala.deser.TupleDeserializer$$Lambda$139/00000000249C9820.apply(Unknown Source) [info] at scala.collection.immutable.Vector1.map(Vector.scala:1872) [info] at scala.collection.immutable.Vector1.map(Vector.scala:375) [info] at com.fasterxml.jackson.module.scala.deser.TupleDeserializer.deserialize(TupleDeserializerModule.scala:45) [info] at com.fasterxml.jackson.module.scala.deser.TupleDeserializer.deserialize(TupleDeserializerModule.scala:10) [info] at com.fasterxml.jackson.databind.deser.std.CollectionDeserializer._deserializeFromArray(CollectionDeserializer.java:347) [info] ... 29 more
具体的には、msgpack-javaでscala.math.BigDecimalをデシリアライズする時に、エラーが発生していました。このエラーを確認した上で、当時としてはmsgpack4zを採用可能として、msgpack-javaについては、課題を認知しながらも放置した状態としていました。
不具合に関するIssues調査
今回、この不具合が変わらず発生している状況を確認した上で、コミュニティに認知されたIssuesとなっているか再確認しました。具体的には、IssuesをBigDecimalというキーワードと、今回発生したエラーメッセージの中からJsonMappingExceptionというキーワードの検索をしました。しかし、当該の問題が取り上げられていることは確認できませんでした。
不具合の修正方法
ここで、改めてコード上で問題となる箇所の特定と、考えられる修正方法の検討を開始しました。主には、例外を吐く直前ではどのような処理コードを通り、どうあれば正常に動作するか調査をしました。具体的な例外を吐く箇所は、BigDecimalのデシリアライズ処理で呼ばれているgetTextというインタフェースの実装内部で、数値を文字列化する想定がされていないことが原因でした。今回の修正としてはインタフェースを提供するライブラリ（jackson-module-scala）と具象実装を提供するライブラリ（msgpack-java）の組み合わせで発生する問題であったため、どちらにも修正パッチを作ることが考えられました。そのため、具体的な修正をする場合の提案実装をしたブランチを自身のforkしたプロジェクト上で作り、それぞれにIssuesを送りました。
Issuesを送る、PR作成、そしてマージ
それぞれに送ったIssuesが以下です。
送ったIssuesのうちjackson-module-scalaの方には、その日のうちにコメントが付きました。
https://github.com/FasterXML/jackson-module-scala/issues/458#issuecomment-664845092
これはいただいたコメントを読んでから気づいたことでしたが、私の提案した修正により、既存のユニットテストを壊してしまっていました。そのため、この方法はjackson-module-scalaを既に使用しているユーザーに影響を与えるため、受け入れられないという内容でした。このコメントをもらってから、この問題はmsgpack-javaの方の修正が取り込まれなければいけないことが見えてきました。ただ、今回の問題がScala側のライブラリとの組み合わせの問題であるために、Java側のライブラリの修正をすることが許容されるかという懸念が湧いてきました。それからしばらくして、Java側のライブラリの作者からもIssueに対するコメントがついていました。
https://github.com/msgpack/msgpack-java/issues/526#issuecomment-668080211
内容としては、BigDecimalでそんな問題は発生しないというものでしたが、これは私の説明が不足していたことで認識の齟齬を発生させていたためでした。改めて、この問題がjava.math.BigDecimalではなく、scala.math.BigDecimalで発生する問題であることを伝えました。
https://github.com/msgpack/msgpack-java/issues/526#issuecomment-668664372
そのコメントをした後で、jackson-module-scalaのコントリビューターから私の修正をフォローするコメントをいただきました。
https://github.com/msgpack/msgpack-java/issues/526#issuecomment-668736359
これについては、msgpack-java側で今回の修正をするモチベーションが低く、このIssueがスルーされてしまう状況をサポートしてくれたものだと感じました。こうしたライブラリのコントリビューター間で、協調をしてくれたことは、とてもありがたい体験でした。
結果、msgpack-javaの方で、以下のPRが作成されました。
https://github.com/msgpack/msgpack-java/pull/527
私の提案した実装についてのユニットテストも追加されたものとなっており、コミュニティの中でのレビューも通り、無事このPRがmasterに取り込まれ、私のIssueもクローズされました。
その後、この修正についてのリリースを出して欲しいというコメントがつけられました。
https://github.com/msgpack/msgpack-java/issues/526#issuecomment-691268437
そして、0.8.21のバージョンで修正リリースがされました。
この修正バージョンを使って、MessagePackのデシリアライズのパフォーマンス比較もできるようになりました。
> jmh/jmh:run -i 20 -wi 20 -f1 .MessagePackDeserializerBenchmark. ... [info] Benchmark Mode Cnt Score Error Units [info] MessagePackDeserializerBenchmark.byJackson thrpt 20 108.086 ± 5.613 ops/ms [info] MessagePackDeserializerBenchmark.byMsgpack4z thrpt 20 154.674 ± 7.616 ops/ms [info] MessagePackDeserializerBenchmark.byJackson avgt 20 0.014 ± 0.006 ms/op [info] MessagePackDeserializerBenchmark.byMsgpack4z avgt 20 0.009 ± 0.001 ms/op
スコアとしては、msgpack4zがシリアライズとデシリアライズのどちらのケースにおいても、優れていることがわかります。しかし、依存するライブラリの親和性を理由にmsgpack-javaを採用したいケースも考えられるため、同様の不具合に悩まされる人にとっての有効な修正として大きな成果に繋がったと思います。
まとめ
今回の取り組みを振り返り、OSSに貢献するということをチームとして取り組むことができたことは、とてもいい体験でした。また、この取り組みは「OSSにPRを送る会」として開始しましたが、現在では「OSSに貢献する会」と改名しました。その意図として、今回挙げた事例のように、私たちはPRを送ったわけではありません。また、OSSのコミュニティに関わりを持つことは、敷居の高さを感じることも少なくなく、そこにPRを送るということになれば尚更です。しかし、単にIssueを送ることからでも、OSSになんらか貢献するという気持ちを普段OSSを利用する者として継続的に持つということの大切さをメンバー間で再認識できました。
こうして、3か月ほどで累積されていた不具合はIssuesとして全て報告済みとなりました。週に2時間の取り組みも、月に2時間へと頻度は下がりました。しかし、現在もメンバーが興味のあるライブラリのIssuesの中から自身が取り組めるものを探したり、それをきっかけにOSSのコードを読む時間として継続しています。
さいごに
計測プラットフォーム部バックエンドチームでは、ZOZOMATをはじめとする計測技術でよりオンラインでの購入体験を向上させたいバックエンドエンジニアを募集しています。ご興味のある方は、以下のリンクからぜひご応募ください！
