Current gecko dispatches tasks of releasing image::Image to main thread at some places. The task was dispatched individually. Then there were cases that the releasing took long time. It increased peak memory usage and caused a problem like Bug 1639280. When main thread is very busy like WebGL, it takes longer time until the Image is released on main thread. If Images are released quickly on main thread, we could reduce peak memory usage.
When SurfaceCache::ReleaseImageOnMainThread() is called, there could be already an ongoing task for releasing Image. It could reduce a duration until release on main thread.
Differential Revision: https://phabricator.services.mozilla.com/D82864
We use FRAME_UPDATE and FRAME_COMPLETE notifications to check if we should resolve/reject img.decode promises. But all of the FRAME_UPDATE/FRAME_COMPLETE notifications for image/test/crashtests/1443232-1.gif come before we change the surface cache entry from a placeholder to available. So RequestDecodeForSize returns false and we don't resolve and we don't get anymore notifications.
In this case the DECODE_COMPLETE notifications comes after we mark the surface cache entry as available so checking if we should resolve for DECODE_COMPLETEs would also work, but it seems reasonable for consumers to expect that they can get a frame after getting a FRAME_COMPLETE notification for it.
Differential Revision: https://phabricator.services.mozilla.com/D70312
--HG--
extra : moz-landing-system : lando
To be able to remove SystemGroup, NS_ReleaseOnMainThreadSystemGroup
needs to have its dependency on SystemGroup removed. Since all
releases using SystemGroup would've released on the main thread anyway
we can safely replace NS_ReleaseOnMainThreadSystemGroup with
NS_ReleaseOnMainThread.
Depends on D64390
Differential Revision: https://phabricator.services.mozilla.com/D67631
--HG--
extra : moz-landing-system : lando
This changeset is a simple find and replace of `MOZ_FALLTHROUGH` and `[[fallthrough]]`.
Unfortunately, the MOZ_FALLTHROUGH_ASSERT macro (to assert on case fallthrough in debug builds) is still necessary after switching from [[clang::fallthrough]] to [[fallthrough]] because:
* MOZ_ASSERT(false) followed by [[fallthrough]] triggers a -Wunreachable-code warning in DEBUG builds
* but MOZ_ASSERT(false) without [[fallthrough]] triggers a -Wimplicit-fallthrough warning in NDEBUG builds.
Differential Revision: https://phabricator.services.mozilla.com/D56440
--HG--
extra : moz-landing-system : lando
This requires replacing inclusions of it with inclusions of more specific prefs
files.
The exception is that StaticPrefsAll.h, which is equivalent to StaticPrefs.h,
and is used in `Codegen.py` because doing something smarter is tricky and
suitable for a follow-up. As a result, any change to StaticPrefList.yaml will
still trigger recompilation of all the generated DOM bindings files, but that's
still a big improvement over trigger recompilation of every file that uses
static prefs.
Most of the changes in this commit are very boring. The only changes that are
not boring are modules/libpref/*, Codegen.py, and ServoBindings.toml.
Differential Revision: https://phabricator.services.mozilla.com/D39138
--HG--
extra : moz-landing-system : lando
Currently it's completely unclear at use sites that the getters for `once`
static prefs return the pref value from startup, rather than the current pref
value. (Bugs have been caused by this.) This commit improves things by changing
the getter name to make it clear that the pref value obtained is from startup.
This required changing things within libpref so it distinguishes between the
"base id" (`foo_bar`) and the "full id" (`foo_bar` or
`foo_bar_DoNotUseDirectly` or `foo_bar_AtStartup` or
`foo_bar_AtStartup_DoNotUseDirectly`; the name used depends on the `mirror` and
`do_not_use_directly` values in the YAML definition.) The "full id" is used in
most places, while the "base id" is used for the `GetPrefName_*` and
`GetPrefDefault_*` functions.
(This is a nice demonstration of the benefits of the YAML file, BTW. Making
this change with the old code would have involved adding an entry to every
single pref in StaticPrefList.h.)
The patch also rejigs the comment at the top of StaticPrefList.yaml, to clarify
some things.
Differential Revision: https://phabricator.services.mozilla.com/D38604
--HG--
extra : moz-landing-system : lando
gfxPrefs Live preferences are almost identical to StaticPrefs.
We leave aside for now those that set a custom change callback as this feature isn't yet supported in StaticPrefs.
Differential Revision: https://phabricator.services.mozilla.com/D31256
--HG--
extra : moz-landing-system : lando
gfxPrefs Live preferences are almost identical to StaticPrefs.
We leave aside for now those that set a custom change callback as this feature isn't yet supported in StaticPrefs.
Differential Revision: https://phabricator.services.mozilla.com/D31256
--HG--
extra : moz-landing-system : lando
gfxPrefs Live preferences are almost identical to StaticPrefs.
We leave aside for now those that set a custom change callback as this feature isn't yet supported in StaticPrefs.
Differential Revision: https://phabricator.services.mozilla.com/D31256
--HG--
extra : moz-landing-system : lando
The size was originally incremented in AnimationFrameBuffer::Insert
however if an animation was reset before we finished decoding, it would
count some frames twice in the counter. Now we increment it inside
InsertInternal, where AnimationFrameDiscardingQueue can make a more
informed decision on whether the frame is a duplicate or not.
Additionally we now fail explicitly when we insert more frames on
subsequent decodes than the original decoders. This will help avoid
getting out of sync with FrameAnimator.
Differential Revision: https://phabricator.services.mozilla.com/D13464
WebRender takes longer than OMTP to release its hold on the current
frame. This is because it is in a separate process and holds onto the
surface in between rendering frames, rather than getting a reference for
each repaint. This patch makes us less aggressive about taking the most
recent surface placed in the recycling queue out to avoid blocking on
waiting for the surface to be released.
Differential Revision: https://phabricator.services.mozilla.com/D10903
Redecode errors break the state machine of FrameAnimator, since the
decoder and the animation state are now out of sync. Going forward this
tight coupling should be eliminated, as the decoder will produce full
frames and the animator can just take the current frame without worrying
about its relative position. For the moment, we should just not reset an
animation if it hit a redecode error (likely due to OOM) just like how
we already stop advancing the animation before the reset.
Differential Revision: https://phabricator.services.mozilla.com/D11046
This is what we have been working towards in all of the previous parts
in the series. This subclasses AnimationFrameDiscardingQueue to save the
discarded frames for recycling by the decoder, if the frame is marked as
supporting recycling.
Differential Revision: https://phabricator.services.mozilla.com/D7516
AnimatedFrameDiscardingQueue subclasses AnimationFrameBuffer to allow a
cleaner abstraction over the behaviour change when we cross the
threshold of too high a memory footprint for an animated image. The next
patch will build on top of this to provide an abstraction to reuse the
discarded frames.
Differential Revision: https://phabricator.services.mozilla.com/D7515
This patch makes AnimationSurfaceProvider use the new abstractions
provided by AnimationFrameBuffer and AnimationFrameRetainedBuffer to
provide storage and lifetime management of decoders and the produced
frames. We initially start out with an implementation that will just
keep every frame forever, like our historical behaviour. The next patch
will add support for discarding.
Differential Revision: https://phabricator.services.mozilla.com/D7514
When blending full frames off the main thread, FrameAnimator no longer
requires access to the raw data of the frame to advance the animation.
Now we only request a RawAccessFrameRef for the current/next frames when
we have discovered that we need to do blending on the main thread.
In addition to avoiding the mutex overhead of RawAccessFrameRef, this
will also facilitate potentially optimizing the surfaces for the
DrawTarget for individual animated image frames.
Differential Revision: https://phabricator.services.mozilla.com/D7506
At present, surface providers roll up all of their individual surfaces
into a single reporting unit. Specifically this means animated image
frames are all reported as a block. This patch removes that
consolidation and reports every frame as its own SurfaceMemoryReport.
This is important because each frame may have its own external image ID,
and we want to cross reference that with what we expect from the GPU
shared surfaces cache.
At present, surface providers roll up all of their individual surfaces
into a single reporting unit. Specifically this means animated image
frames are all reported as a block. This patch removes that
consolidation and reports every frame as its own SurfaceMemoryReport.
This is important because each frame may have its own external image ID,
and we want to cross reference that with what we expect from the GPU
shared surfaces cache.
This was done automatically replacing:
s/mozilla::Move/std::move/
s/ Move(/ std::move(/
s/(Move(/(std::move(/
Removing the 'using mozilla::Move;' lines.
And then with a few manual fixups, see the bug for the split series..
MozReview-Commit-ID: Jxze3adipUh
DrawableSurface only exposes DrawableFrameRef to its users. This is
sufficient for the drawing related code in general, but FrameAnimator
really needs RawAccessFrameRef to the underlying pixel data (which may
be paletted). While one can get a RawAccessFrameRef from a
DrawableFrameRef, it requires yet another lock of the imgFrame's mutex.
We can avoid this extra lock if we just allow the callers to get the
right data type in the first place.
We can discard frames from an animated image if the memory footprint
exceeds the threshold. This will cause us to redecode frames on demand
instead. However decoders can fail to produce the same results on
subsequent runs due to differences in memory pressure, etc. If this
happens our state can get inconsistent. In particular, if we keep
failing on the first frame, we end up in an infinite loop on the decoder
thread.
Since we don't have the owning image to signal, as we had to release our
reference to it after the first pass, we can do little but stop decoding.
From the user's perspective, the animation will come to a stop.
We can discard frames from an animated image if the memory footprint
exceeds the threshold. This will cause us to redecode frames on demand
instead. However decoders can fail to produce the same results on
subsequent runs due to differences in memory pressure, etc. If this
happens our state can get inconsistent. In particular, if we keep
failing on the first frame, we end up in an infinite loop on the decoder
thread.
Since we don't have the owning image to signal, as we had to release our
reference to it after the first pass, we can do little but stop decoding.
From the user's perspective, the animation will come to a stop.
When we shutdown the decode pool threads, it does not do a simple join
with the main thread. It will actually process the main thread event
loop, which can cause a bad series of events. The refresh tick could
still be running and advancing our animated images, causing the animated
decoders to continue running, which in turn prevents the decoder threads
from finishing shutting down, and the main thread from joining them.
Now we check on each frame whether or not the decoder should just stop
decoding more frames because the decode pool has started shutdown. If it
has, it will stop immediately.
After decoding the first frame we allocate the second frame, but before it finishes we encounter an error, Decoder::PostError is called it aborts the second frame and decrements the frame count. But AnimationSurfaceProvider::CheckForFrameAtTerminalState just asks for the current frame ref from the decoder (which it never cleared) and inserts that.
The condition that we use from the decoder to decide to report a new frame is mFinishedNewFrame (via TakeCompleteFrameCount), however this doesn't directly correspond to mFrameCount. So we create a new bool on the Decoder to track when there is a frame that we can take.
This didn't cause any problems before but now we have tighter coupling between the list of frames the AnimationSurfaceProvider has and what FrameAnimator expects.
Another possible fix would be to clear the current frame ref in PostError, but the only place we clear the current frame is when we allocate the new frame and we have the mImageData pointer still around that decoders could theorhetically use to do final processing on the last partial frame.
When we need to recreate an animated image decoder because it was
discarded, the animation may have progressed beyond the first frame.
Given that later in the patch series we need FrameAnimator to be driving
the decoding more actively, it simplifies its role by making it assume
the initial state of the decoder matches its initial state. Passing in
the currently displayed frame allows the decoder to advance its frame
buffer (and potentially discard unnecessary frames), such that when the
animation actually wants to advance as it normally would, the decoder
state matches what it would have been if it had never been discarded.
The shared memory handle reporting has been generalized to be an
external handle reporting. This is used for both shared memory, and for
volatile memory (on Android.) This will allow us to have a better sense
of just how many handles are being used by images on Android.
Additionally we were not properly reporting forced heap allocated
memory, if we were putting animated frames on the heap. This is because
we used SourceSurfaceAlignedRawData without implementing
AddSizeOfExcludingThis.
