Skip to content

refactor Kotlin extensions#375

Open
MukjepScarlet wants to merge 1 commit intoJOML-CI:mainfrom
MukjepScarlet:kotlin-refactor
Open

refactor Kotlin extensions#375
MukjepScarlet wants to merge 1 commit intoJOML-CI:mainfrom
MukjepScarlet:kotlin-refactor

Conversation

@MukjepScarlet
Copy link

  1. Make them hide to Java
  2. Make them inline for better performance

1. Make them hide to Java
2. Make them inline for better performance
@Icosider
Copy link

For the second point: This won't have any effect, as inline was added to embed code instead of a lambda. Lookt at Kotlin inline the first part.

Inlining may cause the generated code to grow. However, if you do it in a reasonable way (avoiding inlining large functions), it will pay off in performance, especially at "megamorphic" call-sites inside loops.

@MukjepScarlet
Copy link
Author

inline makes sense to:

  1. Kotlin FunctionN types
  2. Simple enough (basically a redirect or alias)

e.g. (stdlib)

@SinceKotlin("1.1")
@kotlin.internal.InlineOnly
public actual inline fun maxOf(a: Int, b: Int): Int {
    return Math.max(a, b)
}

You can search @Suppress("NOTHING_TO_INLINE") on GitHub. Up to now there are 13k code result, including: okio, kotlinx-datetime and many other libs.

@Astralchroma
Copy link

When used like this inline basically replaces the inline function call with the function the inline function is calling internally, it basically operates as an alias. It won't increase binary size.

The actual performance increase from this is likely very low or none at all, the JVM is probably smart enough to inline both function calls anyway, the most this will do is speed things up a little bit until JIT kicks in.

It's not a bad change, but it's not likely to yield any improvement.

@MukjepScarlet
Copy link
Author

Another purpose is to avoid auto completion like Matrix4fKt to Java code.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants