Skip to content

[RFC] Introduce Kotlin#6826

Closed
julioz wants to merge 2 commits intogoogle:dev-v2from
julioz:jz/convert-single-suite-to-kotlin
Closed

[RFC] Introduce Kotlin#6826
julioz wants to merge 2 commits intogoogle:dev-v2from
julioz:jz/convert-single-suite-to-kotlin

Conversation

@julioz
Copy link
Copy Markdown
Contributor

@julioz julioz commented Jan 3, 2020

Kotlin is becoming an industry-standard for Android development. It is also endorsed by Google as a 1st-class language for writing apps; and even some official documentation and ExoPlayer sample apps are already written in that language.

Many companies from various sizes have already adopted Kotlin and some important libraries on the Android ecosystem start to migrate to Kotlin as well.


This PR is a request-for-comments, mostly from the ExoPlayer core development team, but also open for the general community.

Besides making ExoPlayer's API type-safe with regards to nullability (while still providing Java interop), replacing @IntDefs with sealed/inline classes for better discoverability and many Guava usages in favor of the standard library ones, using Kotlin would also allow us to greatly reduce the API surface: drop a big number of method overloads (that could be replaced for default parameters and copy constructors), all parameter annotations in favour of language-supported named parameters (again with type-checking), etc.
I'm sure there are other benefits that I'm immediately missing to list.

For now, it only introduces the dependency to the Kotlin runtime on the library-core module and only for the test build type (hopefully we agree on making Kotlin available to even more modules and later to production code).
This PR converts a single test case (FormatTest), simple enough to see how a usage of the stdlib removes a Guava dependency (plus the mental overhead of dealing with mutable collections). There would be way greater benefits in converting more interesting test suites, like ExoPlayerTest, for example.

At SoundCloud (and the approach I would propose) our experience was to introduce new code in Kotlin, and only convert Java to Kotlin as we needed to touch it. I'm happy to provide more in-depth details of our experience if that's of interest of the Exo team.

private val initData: List<ByteArray> = listOf(
byteArrayOf(1, 2, 3),
byteArrayOf(4, 5, 6)
)
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Notice no guava dependency anymore, since collections in Kotlin are immutable by default, unless explicitly stated otherwise (in this case it would mean writing mutableListOf).

Copy link
Copy Markdown
Contributor Author

@julioz julioz Jan 3, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, interesting to notice the type-inference for byteArrayOf when passing the list of values, another benefit of using Kotlin over Java.

parcel.setDataPosition(0)
val formatFromParcel = Format.CREATOR.createFromParcel(parcel)
Truth.assertThat(formatFromParcel).isEqualTo(formatToParcel)
parcel.recycle()
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this final block, we could also apply some standard library functions to make the code a bit more concise.

@ojw28
Copy link
Copy Markdown
Contributor

ojw28 commented Jan 3, 2020

In my opinion, there isn't a strong argument for us to start using Kotlin in the core library at this point in time. We (the whole team) spent some time using Kotlin toward the end of last year, and I agree there's a lot to like.

However, forcing app developers who aren't already bundling the Kotlin runtime to do so is very painful. I would argue that it's probably significantly worse for us to do this than for a network stack library, because there are fewer good alternatives (that a developer could switch to) and because streaming media is an area that's evolving very rapidly (which makes it less viable for a developer to freeze on the current version for an extended period of time).

Many companies from various sizes have already adopted Kotlin and some important libraries on the Android ecosystem start to migrate to Kotlin as well.

It's interesting that OkHttp have migrated. That will provide us with a data point to keep an eye on. It should be noted that not all of the feedback has been positive (e.g. some of the responses in square/okhttp#4723).

Besides making ExoPlayer's API type-safe with regards to nullability (while still providing Java interop)

We're already using Checker framework to achieve this with the current Java implementation. The vast majority of the library is already checked, and we hope to complete removal of the remaining exemptions relatively soon.

@ojw28 ojw28 self-assigned this Jan 3, 2020
@julioz
Copy link
Copy Markdown
Contributor Author

julioz commented Jan 3, 2020

@ojw28 Thanks for sharing your opinion and the prompt response, as always.

I'm curious on a few points you raised:

  • What would be the distribution of consumers of ExoPlayer that have Java-only codebases in contrast to mixed Java/Kotlin or Kotlin-only. Would be quite interesting to have that data publicly if you can provide it (on top of that, out of curiosity, of Google's media apps, like YouTube, Google Play Music, which ones are already bundling Kotlin? Universal Android Music Player was rebuilt from the ground up; so arguably there are more up to date Kotlin documentation than Java on that space).
    Here are a few datapoints from 2019 that show big adoption of Kotlin (mixed with Java), especially on Android (which is ExoPlayer's target platform).
    Given these two sets of data, I'm curious to compare what's the biggest populational impact: Java-only consumers or bringing the benefits of Kotlin to mixed-source consumers (my gut feeling says at this point the majority of the "media apps that evolve rapidly" already use Kotlin - I don't have data to support this statement but we could do an unscientific poll on the ExoPlayer slack to give us a rough idea).
  • What exactly is very painful about bundling the Kotlin runtime? Is it because of method count/APK size as described in OkHttp's thread? Proguard and D8/R8 can help slimming the impact in these metrics, stripping out unused code. I'd also like to point that given OkHttp's move to Kotlin sources, that means at least part of the consumers of ExoPlayer already bundle the runtime within their apps; certainly most of the users of the OkHttp extension.

I think the work that has happened including nullability annotations greatly helps Kotlin consumers, but as I listed in the original message, there are more language-features that could make ExoPlayer an even better tool for developers on Android.

@julioz
Copy link
Copy Markdown
Contributor Author

julioz commented Jan 3, 2020

I'd like to add that the proposal in this PR is to include Kotlin in test-sources only for now, which also means it isn't code that will be delivered to the final production APKs of app developers.

It will be a good playground for the maintainer team to get familiar with the extra tools for modeling and API-design; besides making the codebase friendlier for newcomers overall. I have a good example of diff that would have been way less painful to produce given some features that Java unfortunately does not yet provide, like default parameters, value objects and copy constructors.

@dpreussler
Copy link
Copy Markdown

For the general discussion. Maybe one way to approach this is to provide additional artifacts that are based on Kotlin to provide nicer API there?
Junit5 has separate kotlin classes just for this: https://github.com/junit-team/junit5/tree/master/junit-jupiter-api/src/main although part of the main artifact.
They would only be used from Kotlin.
Android Jetpack provides kotlin extensions: https://developer.android.com/kotlin/ktx as optional libraries
This could be a way to go for Exoplayer too.

Usage stats then could show how many developers use which API to know when Exoplayer can fully switch. Kotlin is already the "preferred way" of writing Android apps for Google, so Java becomes the fallback solution.

@ojw28
Copy link
Copy Markdown
Contributor

ojw28 commented Jan 3, 2020

What would be the distribution of consumers of ExoPlayer that have Java-only codebases in contrast to mixed Java/Kotlin or Kotlin-only. Would be quite interesting to have that data publicly if you can provide it (on top of that, out of curiosity, of Google's media apps, like YouTube, Google Play Music, which ones are already bundling Kotlin? Universal Android Music Player was rebuilt from the ground up; so arguably there are more up to date Kotlin documentation than Java on that space).

I'm not able to share that information here, sorry. Although I'd suggest that results from a survey of developers who use Kotlin as one of their primary programming languages is probably not the most reliable of indicators for measuring Kotlin adoption.

We will continue to look at the language mix being used by app developers who also depend on ExoPlayer, and this will continue to be an input into deciding when it makes sense for us to start using Kotlin within the library.

Universal Android Music Player was rebuilt from the ground up; so arguably there are more up to date Kotlin documentation than Java on that space).

Kotlin demo apps are great. We might convert some of ours as well. We're committed to making ExoPlayer easy to use from Kotlin (hence all of the nullness annotation work we've undertaken). It's unclear how this is directly related to what language we develop the core library in, however.

What exactly is very painful about bundling the Kotlin runtime? Is it because of method count/APK size as described in OkHttp's thread? Proguard and D8/R8 can help slimming the impact in these metrics, stripping out unused code. I'd also like to point that given OkHttp's move to Kotlin sources, that means at least part of the consumers of ExoPlayer already bundle the runtime within their apps; certainly most of the users of the OkHttp extension.

Yes, that's the main pain point. And yes, it's correct that those using OkHttp 4 will already be pulling it in. That's why OkHttp 4 adoption is a good data point for us to keep an eye on. Regarding the ExoPlayer OkHttp extension specifically, I can say that adoption is tiny relative to adoption of the core library.

I'd like to add that the proposal in this PR is to include Kotlin in test-sources only for now, which also means it isn't code that will be delivered to the final production APKs of app developers.

There's no technical reason not to use Kotlin for tests, but I don't agree that it's beneficial to do so. It effectively requires developers to understand two languages before they can do anything, rather than one. I think there needs to be a bigger payoff than being able to use Kotlin syntax in test code for that to be worthwhile.

It will be a good playground for the maintainer team to get familiar with the extra tools for modeling and API-design; besides making the codebase friendlier for newcomers overall.

This argument gets floated a lot for tooling that might make the development team's job a bit easier. You can make a similar argument for insert your favourite dependency injection framework here, AutoValue, other annotation processors and so on. If you're developing an app then this argument often makes sense, but the same is not so true when you're developing a widely used open source library that a large number of developers from different companies need to dip in and out of. The more tools we use, the harder it is for developers to do this. Some development inefficiency on our side is a price worth paying for keeping the code accessible to the widest group of external developers. This doesn't mean that we we'll never adopt additional tooling, but it does mean that the bar for adoption is significantly higher than it would be were we developing an app.

I have a good example of diff that would have been way less painful to produce given some features that Java unfortunately does not yet provide, like default parameters, value objects and copy constructors.

The Format class has grown into a bit of a mess, and adding a new field is indeed very painful at the moment. However it's a stretch to say that Kotlin features are important for fixing it. There are ways to rewrite Format in Java that would also solve the problem. We also had an internal discussion where AutoValue was mooted as the solution.

Android Jetpack provides kotlin extensions: https://developer.android.com/kotlin/ktx as optional libraries. This could be a way to go for Exoplayer too.

We've considered doing this, and I agree that it seems like a much better approach. We didn't see any compelling reasons or use cases to prioritize a more in depth investigation, however.

@swankjesse
Copy link
Copy Markdown

If you changed APIs to Kotlin, callers would benefit from named & default parameters. That’s amazing!

@julioz julioz closed this Feb 29, 2020
@google google locked and limited conversation to collaborators Apr 30, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants