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

Functions that use lambdas should be inlined where possible #76

Open
NightEule5 opened this issue Jan 19, 2021 · 3 comments
Open

Functions that use lambdas should be inlined where possible #76

NightEule5 opened this issue Jan 19, 2021 · 3 comments

Comments

@NightEule5
Copy link

NightEule5 commented Jan 19, 2021

I noticed that functions using lambdas aren't utilizing Kotlin's inline keyword. This could have avoidable performance impacts.

Take, for example, this function:

fun csvReader(init: CsvReaderContext.() -> Unit = {}): CsvReader {
    val context = CsvReaderContext().apply(init)
    return CsvReader(context)
}

The JVM doesn't have higher-order functions, so Kotlin must generate a class (a "SAM type") with the lambda's code in a single method. This doesn't matter too much if Kotlin can generate a singleton object, but in this case it can't, as CsvReaderContext() is captured in the closure. So, every time this function is invoked the lambda's class must be instantiated with CsvReaderContext() in a field, invoked, then garbage collected right after. (Correct me if I'm wrong)

*Small correction: this is a bad example. Looking at the bytecode, the reader context is passed as a method parameter.

I'm not sure how this works on other platforms, but on the JVM this impacts performance.

To avoid this, Kotlin provides inline functions, which inline the function's bytecode into where it's used. This mitigates the performance issues above at the expense of bytecode size being larger. If internal types or functions are used, you can add the @PublishedApi annotation to allow them to be accessed by the function, which makes whatever it's applied to public in the bytecode but obeyed by Kotlin.

A more impactful example would be the open functions in CsvReader.kt and CsvWriter.kt

Now, whether this is that big of a deal in this case is debatable.

@doyaaaaaken
Copy link
Owner

Thank you very much for your useful remarks.
That was a perspective that had been missing.

I'm not familiar with the behavior of inline functions, so I'll look into it right away and figure out how to handle it.

@NightEule5
Copy link
Author

NightEule5 commented Jan 21, 2021

Thanks for the reply. I forked the repository, but couldn't get the open functions to work with the tests when inlined ("Wrong bytecode generated" error). Admittedly, my experience with tests is limited though.

@doyaaaaaken
Copy link
Owner

@NightEule5
I tried too and got an error.
Though, Inline function makes code performance better, but funcitons like csvReader , open are not often called. So the performance affect is very small.

It seems to work well with just the diffs on the following three files in your branch, so I'd like to merge only this fix this time.

  • build.gradle.kts
  • CsvReaderDsl.kt
  • CsvWriterDsl.kt

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants