search

twitter / summingbird

Streaming MapReduce with Scalding and Storm
https://twitter.com/summingbird
Apache License 2.0
2.14k stars 267 forks source link

memoize some Producer hashCodes #754

Closed johnynek closed 6 years ago

johnynek commented 6 years ago

This is the same class of bugfix as:

https://github.com/stripe/dagon/pull/19 https://github.com/twitter/scalding/pull/1789

codecov-io commented 6 years ago

Codecov Report

:exclamation: No coverage uploaded for pull request base (develop@39d80c1). Click here to learn what that means. The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##             develop     #754   +/-   ##
==========================================
  Coverage           ?   72.29%           
==========================================
  Files              ?      154           
  Lines              ?     3746           
  Branches           ?      210           
==========================================
  Hits               ?     2708           
  Misses             ?     1038           
  Partials           ?        0
Impacted Files Coverage Δ
.../main/scala/com/twitter/summingbird/Producer.scala 78.57% <100%> (ø)

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 39d80c1...c83404b. Read the comment docs.

non commented 6 years ago

LGTM

ttim commented 6 years ago

Looks good but I would be happy if there any way to put this memoization on the top level Producer trait.

johnynek commented 6 years ago

@ttim if you can think of a way, let me know. This fixed one class of exponential time bugs we were seeing