Instrument packet-loop wakeups and CPU time (#653) - #689
Draft
kasnder wants to merge 1 commit into
Draft
Conversation
Routing system apps through the tun is suspected to cost battery through wakeup frequency rather than throughput, but there was no way to check that on a device: the only figures the app surfaced were session counts and MB/s, and MB/s is exactly the metric that stays low while background chatter keeps waking the loop. Add always-on counters to handle_events(): loop iterations, epoll_wait calls, wakeups vs. timeouts, polls capped to EPOLL_MIN_CHECK, ready events split by source, tun packets/bytes, wall time spent scanning sessions vs. dispatching events, and the loop thread's CPU time. CPU time comes from CLOCK_THREAD_CPUTIME_ID, so it excludes time blocked in epoll_wait and is comparable across runs of different lengths; it is sampled at most once a second (plus once at loop exit) because that clock is not served from the vDSO. The remaining per-iteration cost is a few relaxed atomic adds and three vDSO CLOCK_MONOTONIC reads. Attribute work per UID where the native side actually knows one: new sessions (handle_ip resolves the UID only for those) and session-socket events. A fixed 128-slot table counts overflow instead of evicting, so a full table degrades into "some traffic unattributed" rather than a skewed split. Java classifies UIDs as system or user from Rule.system, matching what the VPN builder excludes when system apps are not routed. The report is exposed through ServiceSinkhole.dump(), i.e. adb shell dumpsys, and a one-line summary goes to logcat whenever the loop stops. No new preference: a diagnostic nobody enables measures nothing, and an expert knob would collide with the philosophy in AGENTS.md section 4. docs/battery-packet-loop-measurement.md documents what is measured, the limits of per-UID attribution, how to run a routed vs. excluded comparison, and the trade-off that stays either way (excluding system apps is friendlier to battery but conflicts with "Block connections without VPN" and leaves trackers in system apps unblocked). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KbwdXJUvWYvzb1WmZJkkQZ
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Addresses the measurement half of #653. Items 2 and 3 of that issue (one clear toggle, documented trade-off) already landed in #656, so this PR builds the instrumentation the investigation actually needs.
Why
The hypothesis in #653 is that routing system apps costs battery through wakeup frequency, not throughput. Nothing in the app could test that: the only figures surfaced were session counts and MB/s — and MB/s is exactly the metric that stays low while background chatter from Play Services and friends keeps the packet loop out of deep idle.
What is measured
Counters added to
handle_events()(session.c), backed bystruct loop_statsin a newloopstats.c:epoll_waitcalls, wakeups vs. timeouts, polls capped toEPOLL_MIN_CHECKCLOCK_THREAD_CPUTIME_IDwakeups/handcpu s/hare the two headline metrics. The CPU clock only advances while the thread runs, so it already excludes time blocked inepoll_waitand is comparable between runs of different lengths — and unlikebatterystats, it is not subject to the VPN-UID mis-attribution noted inAGENTS.md§4.Counters reset per native run (
jni_start), so a routed run and an excluded run never mix.Cost
Per iteration: a handful of relaxed atomic adds and three vDSO
CLOCK_MONOTONICreads — orders of magnitude below theepoll_waitsyscall they accompany.CLOCK_THREAD_CPUTIME_IDis not in the vDSO, so it is sampled at most once a second (plus once at loop exit, on the loop thread) rather than per iteration.Per-UID attribution
handle_ip()resolves a UID only for new sessions, so that is where upstream attribution happens; downstream attribution comes from the session that owns a ready socket. Packets of an established session carry no UID, sotun_packetsstays a global total — documented rather than faked. A fixed 128-slot table incrementsuid_overflowinstead of evicting entries, so a full table degrades into "some traffic unattributed" rather than a silently skewed split, and the report prints the overflow count whenever it is non-zero.System-vs-user classification uses
Rule.system(cached inServiceSinkhole.mapUidSystem), which matches exactly what the VPN builder excludes when system apps are not routed.How to read it
dumpsyscosts nothing when nobody calls it, which is why there is no periodic sampler and no UI. A one-line summary also goes to logcat every time the loop stops, so a run is not lost if the VPN restarts first.No new preference. A diagnostic nobody turns on measures nothing, and an expert knob would collide with
AGENTS.md§4 constraint 1.Documentation
docs/battery-packet-loop-measurement.mdcovers what each counter means, the limits of per-UID attribution, and a comparison protocol ordered by confidence (within-run system share → screen-off idle A/B → correlate withbatterystats), including the confounds to hold fixed (wg_enabled, blocking mode). It also restates the trade-off that survives whatever the measurement concludes: excluding system apps is friendlier to battery, but conflicts with Android's "Block connections without VPN" and leaves trackers inside system apps unblocked.AGENTS.mdlinks to it from §2 and §4.Verification
:app:externalNativeBuildGithubDebug— passes, no new warnings from the touched C files;Java_..._jni_1get_1loop_1statsconfirmed exported inlibnetguard.so:app:compileGithubDebugJavaWithJavac— passes:app:testGithubDebugUnitTest— 173 tests, 0 failures, including 10 newPacketLoopStatsTestcases (wire format, truncated-array tolerance, per-hour rates, system/user split, ordering, overflow reporting):app:lintGithubDebug— 6 errors, all pre-existingStringFormatInvalidin translation files; none from these changes-1as a valid UID distinct from a free slot, overflow-without-eviction, reset) was additionally exercised with a host-compiled harnessNot verified on a device — the measurement run itself is the next step, and is what the numbers in #653 should come from.
Generated by Claude Code