search

MarioAriasC / funKTionale

Functional constructs for Kotlin
915 stars 71 forks source link

Inlining Option functions #9

Closed mustafin closed 7 years ago

mustafin commented 8 years ago

Inline Option functions to remove overhead on creating lambdas.

MarioAriasC commented 8 years ago

Give me a good reason or use case for this. Something that will improve usage or solves a problem

mustafin commented 8 years ago

As documentation for inlining says: "non-local control flow is not allowed in the lambdas". So we can't call it in flatMap while inilining, another fix is to mark lambda with crossinline but it is better for performance to inline whole function.

MarioAriasC commented 8 years ago

Is the performance gain important enough to justify make us maintain more code in the long term?

It could be tested with JMH or other similar tool, but seems like a rabbit hole. In that case other functions could be inlined too

mustafin commented 8 years ago

You can look at Scala's implementation of Option.map @inline final def map[B](f: A => B): Option[B] = if (isEmpty) None else Some(f(this.get)) It is not using flatMap yet it is short and simple. Again from docs: "Inlining may cause the generated code to grow, but if we do it in a reasonable way (do not inline big functions) it will pay off in performance, especially at “megamorphic” call-sites inside loops"

MarioAriasC commented 8 years ago

Homework

  1. Delete the import; isn't needed (If is failing in your installation, then you have a problem)
  2. Reformat your code
  3. Change title and description, explaining why is need it and the potential benefits.
  4. Check if there is any other function that could be inlined in this file
MarioAriasC commented 8 years ago

👍