Skip to content
This repository has been archived by the owner on Jan 20, 2022. It is now read-only.

Change depthFirstOf to use List instead of Vector #323

Merged
merged 1 commit into from
Oct 23, 2013

Conversation

ryanlecompte
Copy link
Contributor

I noticed that depthFirstOf is always called with a toList following it. I performed a quick Thyme benchmark between using Vector for appends vs. constructing a List and finally calling .reverse on it:

scala> th.pbench { (0 to 1000).foldLeft(List.empty[Int]) { case (xs, i) => i :: xs }.reverse }
Benchmark (163820 calls in 4.485 s)
  Time:    20.60 us   95% CI 19.58 us - 21.62 us   (n=20)
  Garbage: 6.519 us   (n=423 sweeps measured)

scala> th.pbench { (0 to 1000).foldLeft(Vector.empty[Int]) { case (v, i) => v :+ i } }
Benchmark (81900 calls in 4.628 s)
  Time:    42.28 us   95% CI 40.25 us - 44.31 us   (n=20)
  Garbage: 13.65 us   (n=580 sweeps measured)

Also, later calling toList on the Vector is probably not very cheap. I am not sure how often depthFirstOf gets called in typical summingbird workflows, but I figured it couldn't hurt. summingbird-core tests pass on my machine with these updates.

@sritchie
Copy link
Collaborator

Woah, most interested in this: https://github.com/Ichoran/thyme

@ryanlecompte
Copy link
Contributor Author

Thyme is awesome! I use it all the time.

@johnynek
Copy link
Collaborator

Niiiice! I'd love pull reqs like this that keep everything as performant as possible.

johnynek added a commit that referenced this pull request Oct 23, 2013
Change depthFirstOf to use List instead of Vector
@johnynek johnynek merged commit adf0088 into twitter:develop Oct 23, 2013
snoble pushed a commit to snoble/summingbird that referenced this pull request Sep 8, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants