Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Chain.map seems not optimized #2959

Closed
jeongwoo-ji opened this issue Jul 24, 2019 · 2 comments · Fixed by #2995
Closed

Chain.map seems not optimized #2959

jeongwoo-ji opened this issue Jul 24, 2019 · 2 comments · Fixed by #2995
Labels
good first issue Issues that are easier to take on for first time contributors help wanted

Comments

@jeongwoo-ji
Copy link

Current implementation of Chain.map is

final def map[B](f: A => B): Chain[B] =
  fromSeq(iterator.map(f).toVector)

Why it isn't implemented like this?

final def map[B](f: A => B): Chain[B] = this match {
  case Wrap(seq) => fromSeq(seq.map(f))
  case _         => fromSeq(iterator.map(f).toVector)
}

I think the problem is:

val x: List[Int] = List.fill(n)(0)
Chain.fromSeq(x).toList // takes O(1)
Chain.fromSeq(x)
  .map(identity) // takes O(n). its not a problem.
  .toList        // takes O(n)!! I think it could be O(1).
@kailuowang
Copy link
Contributor

I agree that your optimization makes sense. Would you like to create a PR?

@kailuowang kailuowang added good first issue Issues that are easier to take on for first time contributors help wanted labels Jul 24, 2019
@gagandeepkalra
Copy link
Contributor

I've just added PR for this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Issues that are easier to take on for first time contributors help wanted
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants