⚡ Optimize VPN Service Startup Performance - #211
Conversation
…ckground thread - Moved blocking calls (Yuhaiin.getTunAddress, configure, start) to a coroutine on Dispatchers.IO. - Ensured startForeground is called early on the main thread for Android compliance. - Added @volatile to state and mInterface for thread safety. - Updated stop() to handle the CONNECTING state. - Managed coroutine lifecycle with serviceScope and onDestroy. Co-authored-by: Asutorufa <16442314+Asutorufa@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
Summary of ChangesHello @Asutorufa, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the VPN service's startup reliability and performance by offloading time-consuming operations from the main thread. By leveraging Kotlin Coroutines, the service can now initialize critical components asynchronously, thereby avoiding potential Application Not Responding (ANR) errors and ensuring that the foreground service notification is displayed promptly, aligning with Android's strict service lifecycle policies. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request successfully optimizes the VPN service startup by moving blocking I/O and JNI calls in onStartCommand to a background thread, which is a great improvement for performance and stability, preventing potential ANR errors. My review includes a critical fix for a resource leak in the new error handling logic and a suggestion to apply similar non-blocking principles to the stop() method to prevent ANRs during shutdown as well.
- Offloaded blocking I/O and VPN initialization to Dispatchers.IO. - Ensured early startForeground call for Android compliance. - Added @volatile and state guards for thread-safe asynchronous startup. - Handled CancellationException to prevent false-positive error logs. - Updated stop() to handle interruptions during the CONNECTING phase. - Addressed PR feedback regarding resource leak prevention and logging. Co-authored-by: Asutorufa <16442314+Asutorufa@users.noreply.github.com>
💡 What: The optimization implemented
This change moves blocking I/O and JNI calls in
YuhaiinVpnService.onStartCommandto a background thread using Kotlin Coroutines. It also ensures thatstartForegroundis called immediately on the main thread to comply with Android's foreground service requirements.🎯 Why: The performance problem it solves
Previously, the VPN service performed several blocking I/O operations (like reading from
Store) and JNI calls on the main thread duringonStartCommand. This could lead to Application Not Responding (ANR) errors and crashes, especially on Android 8+ where failing to callstartForegroundwithin a few seconds of starting the service results in a crash.📊 Measured Improvement:
While it was impractical to measure the exact millisecond improvement in this environment due to build and network constraints, moving blocking I/O off the main thread is a standard Android performance optimization that prevents main-thread stalls and ANRs. The
onStartCommandnow returns almost immediately, allowing the system to proceed while the VPN initializes in the background. Additionally, callingstartNotificationearlier makes the service more robust against "Service.startForeground() not called" errors.PR created automatically by Jules for task 3314752751189640977 started by @Asutorufa