Repository navigation
Time out slow image captions, and end each caption request once - #145
Merged
Merged
Conversation
Image captioning gives up on a caption that takes more than 5 seconds,
says what it has so far while the image is still focused, and moves on
to the next screenshot. The timeout was set with
System.currentTimeMillis() on a Handler, which counts uptime instead,
so it was due decades later and never fired. A slow part, such as text
recognition that did not answer, held the next screenshot until its own
10-second timeout.
The timeout now fires after 5 seconds. It used to clear every list,
including screenshot requests that were waiting, such as a Describe
image the user had just asked for; that went unnoticed only because it
never fired. It now ends the slow caption's requests and lets the next
screenshot go ahead. A caption with nothing to do drops its timeout at
once, so that the timeout cannot give up on a Describe image started
within those 5 seconds. Describe image with Gemini or the on-device
model does not use this timeout; it has its own.
A request could also end twice. A screenshot result that came after its
3-second timeout ended its request a second time, and as the list always
removed its first request, it removed the next one while that was still
taking its screenshot, and started the one after it. A result that came
after its caption had been dropped did the same.
Each request now ends once, with a result, an error, its timeout or a
cancel, and whatever comes after that is ignored. The list removes the
request that ended, not whichever is first, and clearing it cancels its
requests and their timeouts. A caption result keeps the screenshot
request it came from, and ends that one when it is complete or dropped.
The time between screenshots, and the time each request takes, are now
measured with SystemClock.uptimeMillis() rather than the wall clock, so
setting the clock back no longer holds the next screenshot back by that
much time.
Limitations: Describe image still stops working after the on-device
model runs out of memory or times out, since GeminiActor ends those
requests as cancelled without telling ImageCaptioner; that needs a fix
of its own. Not tried on a device.
- Request: finish() ends a request once, cancel() ends it without a
result, and its times are in uptime
- RequestList: performNextRequest(Request) removes the request that
ended, clear() cancels the requests, and the time between them is in
uptime
- ScreenshotCaptureRequest: passes itself to its listener, and recycles
a screenshot that comes after the request ended
- CaptionRequest: ignores a result or an error after the request ended
- ImageCaptioner: ends the screenshot request each caption came from,
the 5-second timeout ends only the slow caption, a caption with
nothing to do drops its timeout, and shutdown() cancels all requests
- ImageCaptionerRequestsTest and RequestListTest; without this change,
the tests of a late screenshot and of a slow caption fail
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
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.
Image captioning gives up on a caption that takes more than 5 seconds,
says what it has so far while the image is still focused, and moves on
to the next screenshot. The timeout was set with
System.currentTimeMillis() on a Handler, which counts uptime instead,
so it was due decades later and never fired. A slow part, such as text
recognition that did not answer, held the next screenshot until its own
10-second timeout.
The timeout now fires after 5 seconds. It used to clear every list,
including screenshot requests that were waiting, such as a Describe
image the user had just asked for; that went unnoticed only because it
never fired. It now ends the slow caption's requests and lets the next
screenshot go ahead. A caption with nothing to do drops its timeout at
once, so that the timeout cannot give up on a Describe image started
within those 5 seconds. Describe image with Gemini or the on-device
model does not use this timeout; it has its own.
A request could also end twice. A screenshot result that came after its
3-second timeout ended its request a second time, and as the list always
removed its first request, it removed the next one while that was still
taking its screenshot, and started the one after it. A result that came
after its caption had been dropped did the same.
Each request now ends once, with a result, an error, its timeout or a
cancel, and whatever comes after that is ignored. The list removes the
request that ended, not whichever is first, and clearing it cancels its
requests and their timeouts. A caption result keeps the screenshot
request it came from, and ends that one when it is complete or dropped.
The time between screenshots, and the time each request takes, are now
measured with SystemClock.uptimeMillis() rather than the wall clock, so
setting the clock back no longer holds the next screenshot back by that
much time.
Limitations: Describe image still stops working after the on-device
model runs out of memory or times out, since GeminiActor ends those
requests as cancelled without telling ImageCaptioner; that needs a fix
of its own. Not tried on a device.
result, and its times are in uptime
ended, clear() cancels the requests, and the time between them is in
uptime
a screenshot that comes after the request ended
the 5-second timeout ends only the slow caption, a caption with
nothing to do drops its timeout, and shutdown() cancels all requests
the tests of a late screenshot and of a slow caption fail