-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
Timezone handling changes in the new scheduler + Rust #5805
Comments
Hey @dae ! Yeah...port to Java is where I'd go. My main gig right now is react-native (reactxp on top of it actually) which unavoidably uses 3 separate toolchains to get anything done and it's a real beast. The payoff is iOS/web/android support from one codebase but since AnkiDroid is android only, I would avoid multiple toolchains if ever possible now that I've felt the multi-year support burden Regardless I'll look at it because I'd like to see some Rust code (never checked Rust out, actually), and I'll mark it for high priority attention though - I try to stay on top of ecosystem stuff as much as possible of course So far @hssm has been our primary V2 champion - do you have any time for examination + porting here? No pressure of course, just curious Cheers all |
I won't have a chance to look into it for another ~2 weeks, so if it's urgent then maybe someone else would like to take a crack at it. |
@dae |
This comment has been minimized.
This comment has been minimized.
I'm not sure I've ever used a cross-platform mobile app that I liked. Having a JNI wrapper around a shared backend with a native Android frontend makes the most sense to me. I don't think it really matters what language the backend is written in, the point being that it's actively maintained by Damien and shared across all clients at the same time. |
This comment has been minimized.
This comment has been minimized.
Hey @Arthur-Milchior - do you have any interest in giving this port of the new timezone cutoff code a try? This is the last thing with no PR that I want to get into 2.10 - I think I am going to bump the minimum SDK version for 2.11 so I want we get it out to the widest possible audience before moving forward. As soon as we have a PR merged for this I can cut the branch and open things up for your other changes in 2.11alpha |
I may try to take a look. Probably in a few day. I make no promise, but that seems really doable |
Okay - I was just pinged about this, I am sorry we are holding up progress in this case. We have had related discussions and are all planning on adopting Rust via JNI and re-using libanki fully in the future, but despite being excited about that idea, it's a pretty big change. So that this is not blocking Desktop then, I believe we need to port this change to AnkiDroid in Java |
The nice thing about this code is that it's small and isolated - it can be used without buying into the rest of the backend infrastructure, and if major issues are found with using Rust in AnkiDroid, it's easy to back out - so it makes for a good canary/test case. And it would be a shame to have to port code that may quickly be made redundant. But it really depends on AD's schedule - if a stable release with even a small amount of Rust is still many months away, then perhaps it makes more sense to do it in Java for now. |
Pinging @david-allison-1 as I believe he is the one who have the most idea about whether it'll be possible to have a small part of rust incorporated or not |
Yeah - @david-allison-1 and I chatted briefly about this yesterday on Discord. Nibbling at Rust integration in this way (to validate that the direction works, as a proof-of-concept) does seem sensible. We have to start somewhere right? Perhaps this is the right moment to take the rust lib, wrap this area via JNI and call into it. @dae we need to determine how we are going to integrate the rust library. Do you publish the compiled artifacts in any location that is consumable with an android dependency system (that's gradle for us, with maven under the covers)? Or do we need to have some sort of scripted periodic sync where we can download a compiled artifact? I'm a firm believer in semantic versioning and would like to rely on an "official" compiled artifact with semantic versions and associated release notes guiding us as a consumer as to when it's safe to move forward or not, or what to do in order to forward-port if we're crossing a breaking change version boundary. |
The Rust code is distributed as source code. To use it, AnkiDroid needs to create a small library in Rust that will act as a bridge, providing an interface Java can call into, and routing the data to the Anki Rust code which gets compiled into it. That library then needs to be compiled for the target architectures before it is included in AnkiDroid as a .so file. The example I posted earlier demonstrates this - there is a small rsdroid.so library that provides a Java API, and routes calls into it to the relevant Rust function. The code is compiled into a library targeting the architectures configured in the build.gradle file as part of the build process, so Rust must be installed to build AnkiDroid. If you don't want people to have to install Rust then you could split the .so building into a separate repo and make the .so files available to people, but I'd recommend against doing that, at least for now. Anki's Rust code uses Anki's version number, which is not semantic from an API perspective. But AnkiDroid's Rust bridge can be pointed to a specific git tag or revision for reproducible builds - in the example it targets the 2.1.21 tag for example. ankitects@937c539#diff-3d99f669fd2d4bd63bd75891c3bda548 The example code uses JSON for communicating between Java and Rust, to avoid overcomplicating things. If AD decides to fully buy in to the Rust code in the future, it will probably want to do it the same way the computer version does - there is a protobuf file defining all the method calls and their arguments, and Python API is automatically generated as part of the build process for each of the API calls provided by the library. This makes API changes obvious - there will be compile errors that need fixing. And if you're working with pinned versions, such problems only need to be addressed when someone deliberately changes the version. |
The proof of concept code on your rust branch is very useful. @david-allison-1 I'm totally fine with the toolchain and bridge being at MVP/PoC (Minimum Viable Product / Proof of Concept) level (consuming via source, JSON bridge) but if we do that I'm not sure how to avoid requiring developers install Rust and NDK unless you commit the finished .so files which is not very git-idiomatic, but some people do. To not crush developers with environment complexity for now I'd be fine with committing the .so's even e.g. everything in here seems reasonable https://github.com/kennytm/rust-ios-android The target SDK not working below 21 is not going to fly though - especially since it seems like it should work https://github.com/mozilla/rust-android-gradle#native-apilevel - seems like it has to be related to this somehow since that link implies strongly that it should be possible to go below 21, and that 64bitness is something that breaks at API21 |
Hello 👋, this issue has been opened for more than 2 months with no activity on it. If the issue is still here, please keep in mind that we need community support and help to fix it! Just comment something like still searching for solutions and if you found one, please open a pull request! You have 7 days until this gets closed automatically |
Huzzah, there's a PR for this based on a Rust backend for AnkiDroid, which I'm noting because that's pretty incredible I think. 🏆 @david-allison-1 |
- In corner cases, enabling the new timezone handling later can cause reviews to shift forward or back a day, so it's best to have it on by default. - ankidroid/Anki-Android#5805 has not landed in a stable release yet, but will hopefully not be too far off by the time 2.1.41 is released. - Existing users will be unaffected, as the upgrade prompt in the previous commit asks them if they use AnkiDroid. - Users starting on AnkiDroid will be unaffected, as their collections will still be on V1. - The error message AnkiWeb gives when syncing an older AnkiDroid with the new timezone enabled has been updated to direct users to the preferences screen.
Hi guys, Just a heads up that the next desktop release will display a prominent message recommending people update to the new scheduler. When upgrading, it asks users if they're using AnkiDroid 2.14, and will keep the old timezone handling if they answer yes. Presumably there'll still be some users who choose the wrong option or decide to use AnkiDroid later, so I've also updated the message AnkiWeb sends to 2.14 when the new timezone is enabled, so that users are told they need to change a setting in the preferences of the computer version. I'd originally planned to enable the v2 scheduler + new timezone code on new installs without any prompt, as showing an upgrade prompt on first startup looks a bit silly. But I ended up reverting the change, as I realised it would be too disruptive. So Anki will show a message on first startup, and users will get a chance to confirm if they're using AnkiDroid, which I hope will minimize disruptions, while still allowing us to move forward with the new scheduler migration. The next stable release is tentatively planned for about two weeks, but may be pushed back if any issues crop up. Looking forward to 2.15! |
Thanks for the heads up @dae ! I think we still have 2 issues (perhaps 3) to isolate/fix for the 2.15 release, and for the release process to go through alpha/beta/stable in 2 weeks would require them to be fixed like yesterday or today which is improbable. Only saying this because I'm really looking forward to 2.15 myself but I think we're 3-4 weeks from it if I'm being reasonable / low-pressure / high-probability |
I'm not trying to rush you, just wanted to get things moving a bit, as issues caused by the old scheduler crop up on pretty much a daily basis. 2.1.41 will be a soft release with no update notification to existing users, and the user will be able to confirm if they use/plan to use AnkiDroid when updating, so it should hopefully not generate much in the way of support questions. If it proves to be causing problems, it can be listed below the previous stable release, or even potentially pulled from the website if it's much more disruptive than expected. 2.15 has some big architectural changes, and I don't want you to feel under pressure to push it out prematurely. |
Hi Mike/Tim/all,
Back in December I introduced some changes to the V2 scheduler that address a few issues with the timezone handling that could cause the daily counts to be reset or syncing errors to occur for some users. For the changes to work properly, all clients need to support them, as otherwise they may disagree on how many days have elapsed, causing syncing errors. So for now I've had to make them optional.
Some brief docs are available here: https://anki.tenderapp.com/kb/anki-ecosystem/timezone-handling-changes
These changes have been largely hidden for the last few months while they received more testing, but I think they're stable enough now to make more visible, and I've placed an option in the latest desktop beta's preference screen, warning users that AnkiDroid does not support the change yet.
It would be great if AnkiDroid could support the changes in the future, as they really belong in the core V2 scheduler rather than as a separate option, and I'd like to integrate them before the V2 scheduler drops its beta tag.
Links to the relevant code are included here:
ankitects/anki@131d37d
The ~90 lines of Rust code could be ported to Java like AnkiDroid has done with the Python code up until now, but it may also be worth considering if it's feasible to reuse the existing code via JNI instead. I have a very basic proof of concept showing that it is possible, but a number of issues would need to be worked through before it was a viable solution:
If you'd like to try the POC, please make sure you have rustup installed, and add the basic archs before you try to build:
Code is at https://github.com/ankitects/Anki-Android/commits/rust
I suspect 'port to Java' will be the decision you opt for, but I thought I'd throw it out there anyway :-)
The text was updated successfully, but these errors were encountered: