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

Geoip performance optimization #1047

Merged
merged 4 commits into from
Jan 29, 2024
Merged

Conversation

bitmold
Copy link
Collaborator

@bitmold bitmold commented Dec 21, 2023

Rather than taking geoip and geoip6 which are zipped into the APK and inflating them, then copying all 10MB of them each time OrbotService starts, we now only extract these files and write them to internal storage if they don't exist or the first time the user opens the app after an update. This way every single time OrbotService is subsequently created it doesn't need to do this disk IO.

My concern was that blocking disk IO could somehow be deprioritized when Orbot starts in the background on device boot or power saving mode is turned onto the device. For users who start with always on VPN maybe this will make them enter the tor network faster.

This geoip business hooks into a lightweight preference based system into OrbotApp that could allow for future on "orbot was just updated to a new version" events.

@bitmold bitmold requested a review from n8fr8 December 21, 2023 22:19
@bitmold bitmold added the PLEASE TEST please test and post feedback label Jan 28, 2024
@syphyr
Copy link
Contributor

syphyr commented Jan 28, 2024

This PR is working as expected. I have tested upgrading and also initial installations.
Just to be sure, I added some additional logging to make sure things were happening correctly like this:

--- a/orbotservice/src/main/java/org/torproject/android/service/OrbotService.java
+++ b/orbotservice/src/main/java/org/torproject/android/service/OrbotService.java
@@ -466,6 +466,7 @@ public class OrbotService extends VpnService implements OrbotConstants {
                 // only write out geoip files if there's an app update or they don't exist
                 if (!hasGeoip || !hasGeoip6 || Prefs.isGeoIpReinstallNeeded()) {
                     try {
+                        Log.i("OrbotService", "Installing geoip files");
                         new CustomTorResourceInstaller(this, appBinHome).installGeoIP();
                         Prefs.setIsGeoIpReinstallNeeded(false);
                     } catch (IOException io) { // user has < 10MB free space on disk...

…e files

dont already exist or if the app has been updated

added very lightweight logic in Application subclass to set flags for geoip
and other events if and only if an app update has occured
vestigial v2 onion service local sqlite tables. I doubt any user
still has them anymore. Further, if disk io is deprioritized when
app starts on boot this could add to a longer initialization time.
It never gets printed, people don't need to translate it.
@bitmold bitmold force-pushed the geoip_performance_optimization branch from f1d9bd5 to 246f71d Compare January 29, 2024 02:31
@bitmold
Copy link
Collaborator Author

bitmold commented Jan 29, 2024

Excellent, I'll push your logging commit

@bitmold bitmold merged commit 9ebab6d into master Jan 29, 2024
1 check failed
@bitmold bitmold deleted the geoip_performance_optimization branch January 29, 2024 02:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PLEASE TEST please test and post feedback
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants