Progress updates flood the UI handler, which is the top ANR - #194
Merged
Merged
Conversation
setProgressLines posted a new Runnable for every engine refresh. Each one walks the layout, measures every line and parses it with Html.fromHtml. The engine queued frames faster than the main thread drew them. The queue grew without limit, which is the ANR, and the frames waiting in it are the OutOfMemoryError. Play attributes 7 of 11 ANR clusters and 2 crash clusters to setProgressLinesInternal. Keep the newest lines in a ProgressCoalescer and post one reusable task. A refresh arriving while a task is pending replaces the payload rather than adding a task. That loses nothing, because each frame replaces the whole progress pane, so a frame that is already superseded has nothing left to show. The task takes the payload and disarms in one synchronized step, before it draws. A refresh landing mid-draw therefore finds the coalescer idle and posts a task of its own. So the last frame of a crawl is drawn even when it arrives while its predecessor is still on screen. Handler.post is also checked, because a false return on a dead looper would leave the latch armed and freeze progress for good. Html.fromHtml still runs per line per frame. Coalescing bounds how often, and moving the parse off the main thread is a separate change. Closes #187 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Xavier Roche <roche@httrack.com>
The failed-post branch called take() and dropped the payload. take() promises a payload, so a reader had to work out that dropping it is safe. The payload dropped need not even be the one that caller offered. A refresh landing between the offer and the post's false return replaces it first. disarm() says what the branch means, and take() now uses it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Xavier Roche <roche@httrack.com>
The drawing task reads an empty coalescer as a null payload, so a null frame would be drawn as nothing. Nothing tested the refusal. The rest is shape. The counter is postedTasks, not queued, because the name has to say whose queue it stands for. The schedule count and the frame count are named, and "frame " + i has a helper. startDraw() no longer hides an assertion inside the fake, and the last test drives the fake instead of hand-rolling a second copy of it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Xavier Roche <roche@httrack.com>
armed was true exactly when pending was non-null, so two fields carried one state and only a reading of the code held them in agreement. A probe over every sequence of offer, take and disarm up to length seven found they never disagree. So pending == null now means no task is owed. The null payload that offer refuses stops being a dead guard under that change. A null would read as nothing pending and let a second task be posted over the first, and the javadoc says so. offer returning true meant "you owe a post", which reads backwards against Queue.offer, where true means accepted. offerNeedsPost says which way it runs at the call site. Two elements stay defensive, and each comment now says why it cannot fire. post() refuses only once the Looper quits, and take() answers null only after a disarm that follows a post. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Xavier Roche <roche@httrack.com>
Three summaries opened with a noun phrase, which reads as a javadoc convention and hides that no sentence is there. The class, the field and the test class now start on a verb. The class doc also ran to 34 words in one sentence, so it is two. Take() no longer nests a clause between the two objects of "gives". Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Xavier Roche <roche@httrack.com>
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
1 participant
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.
setProgressLinesposted one Runnable per engine refresh, and each one walks the layout, measures every line and parses it withHtml.fromHtml. The engine produces frames faster than the main thread draws them, so the queue grew without limit.The newest lines now sit in a
ProgressCoalescerbehind one reusable task, and a refresh arriving while a task is pending replaces the payload. Nothing is lost, because each frame replaces the whole progress pane.The task takes the payload and disarms in one synchronized step before drawing. A refresh landing mid-draw then posts a task of its own, so the final frame still reaches the screen. Three mutants of the coalescer each fail the new tests.
Moving
Html.fromHtmloff the main thread is a separate change.Closes #187