Skip to content

fix #1025, cache schema in Avro SchemaProvider#1359

Merged
nevillelyh merged 1 commit intomasterfrom
neville/avro
Sep 12, 2018
Merged

fix #1025, cache schema in Avro SchemaProvider#1359
nevillelyh merged 1 commit intomasterfrom
neville/avro

Conversation

@nevillelyh
Copy link
Contributor

No description provided.

@codecov-io
Copy link

Codecov Report

Merging #1359 into master will decrease coverage by 0.04%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1359      +/-   ##
==========================================
- Coverage   83.26%   83.22%   -0.05%     
==========================================
  Files         125      125              
  Lines        4028     4029       +1     
  Branches      438      463      +25     
==========================================
- Hits         3354     3353       -1     
- Misses        674      676       +2
Impacted Files Coverage Δ
...a/com/spotify/scio/avro/types/SchemaProvider.scala 95.74% <100%> (+0.09%) ⬆️
...y/scio/values/PairSkewedSCollectionFunctions.scala 90.74% <0%> (-3.71%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f4d8874...573a8cc. Read the comment docs.

Copy link
Contributor

@regadas regadas left a comment

Choose a reason for hiding this comment

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

@nevillelyh LGTM, however for this simple case i think you can get slightly better performance with a ConcurrentHashMap.


private[types] object SchemaProvider {

private val m: scala.collection.concurrent.Map[Type, Schema] =
Copy link
Contributor

Choose a reason for hiding this comment

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

🎨 private[this]

@nevillelyh nevillelyh merged commit d045a96 into master Sep 12, 2018
@nevillelyh nevillelyh deleted the neville/avro branch September 12, 2018 14:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants