Skip to content

Update to Kotlin 1.6 and use the new Memory Model#370

Merged
pm47 merged 13 commits intomasterfrom
kotlin-1.6
Sep 23, 2022
Merged

Update to Kotlin 1.6 and use the new Memory Model#370
pm47 merged 13 commits intomasterfrom
kotlin-1.6

Conversation

@pm47
Copy link
Member

@pm47 pm47 commented Sep 19, 2022

The old memory model based on freezing objects that moved between threads caused a lot of issues with Phoenix, especially on iOS. With the upgrade to kotlin 1.6, ktor 2.x and coroutines 1.6 it makes sense to also move to the new model which is much more workable than the old one.

To do so we enable an experimental flag and use the adequate kotlinx coroutine library version.

See https://github.com/JetBrains/kotlin/blob/master/kotlin-native/NEW_MM.md

@pm47 pm47 requested review from dpad85 and sstone September 19, 2022 15:27
@pm47 pm47 marked this pull request as ready for review September 19, 2022 15:27
@pm47 pm47 changed the title Update to Kotlin 1.6 Update to Kotlin 1.6 and use the new Memory Model Sep 23, 2022
@dpad85
Copy link
Member

dpad85 commented Sep 23, 2022

I've added a new commit 1e28d9e that enables the new memory model. For context, see:

So this makes this PR even more impactful!

The reason why we do this now is that the switch to ktor 2.x causes several issues in Phoenix (some freezing APIs are gone, and http calls are done in the background thread, causing unexpected object mutations hence crashes). Rather than fixing those issues in Phoenix it makes more sense to move to the new memory model now, which was anyway bound to happen. Also, with all the other changes coming up, we'll have to test the app thoroughly so it's a good time to do this.

This change has been tested successfully on Android. On iOS most of the app works out of the box, and with @robbiehanson we think we can fix the remaining issues.

dpad85 and others added 11 commits September 23, 2022 16:50
Fixes build issue with the PhoenixCrypto module containing native logic
used in the iosMain.

See https://kotlinlang.org/docs/multiplatform-share-on-platforms.html#use-native-libraries-in-the-hierarchical-structure
…tore submissions with Bitcode embedded. So this should be removed.
…hoenixCrypto project. The linker complains with a warning about 13
Both libraries now use kotlin 1.6.21.
The old memory model based on freezing objects that moved between threads
caused a lot of issues with Phoenix, especially on iOS. With the upgrade
to kotlin 1.6, ktor 2.x and coroutines 1.6 it makes sense to also move
to the new model which is much more workable than the old one.

To do so we enable an experimental flag and use the adequate kotlinx
coroutine library version.

See https://github.com/JetBrains/kotlin/blob/master/kotlin-native/NEW_MM.md
@pm47
Copy link
Member Author

pm47 commented Sep 23, 2022

Rebased and cleaned up, I think we're good to go!

Copy link
Member

@dpad85 dpad85 left a comment

Choose a reason for hiding this comment

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

LGTM!

@pm47 pm47 merged commit 30a82b2 into master Sep 23, 2022
@pm47 pm47 deleted the kotlin-1.6 branch September 23, 2022 16:26
robbiehanson added a commit that referenced this pull request Mar 23, 2023
The old memory model based on freezing objects that moved between threads caused a lot of issues with Phoenix, especially on iOS. With the upgrade to kotlin 1.6, ktor 2.x and coroutines 1.6 it makes sense to also move to the new model which is much more workable than the old one.

To do so we enable an experimental flag and use the adequate kotlinx coroutine library version.

See https://github.com/JetBrains/kotlin/blob/master/kotlin-native/NEW_MM.md

Co-authored-by: dpad85 <5765435+dpad85@users.noreply.github.com>
Co-authored-by: Robbie Hanson <robbiehanson@deusty.com>
Co-authored-by: sstone <fabrice.drouin@acinq.fr>

(cherry picked from commit 30a82b2)
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.

4 participants