Before this patch: surface cache size = min(1GB, system_memory/4)
After this patch: surface cache size = min(32bit ? 1GB : 2GB , system_memory/4)
sizeof(uintptr_t) is the best I can figure out to detect 32bit builds.
Differential Revision: https://phabricator.services.mozilla.com/D45092
--HG--
extra : moz-landing-system : lando
Before this patch: surface cache size = min(1GB, system_memory/4)
After this patch: surface cache size = min(32bit ? 1GB : 2GB , system_memory/4)
sizeof(uintptr_t) is the best I can figure out to detect 32bit builds.
Differential Revision: https://phabricator.services.mozilla.com/D45092
--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
SVG performance with the fallback path with WebRender is very bad. This
patch avoids fallback by always producing a rasterized surface we store
in SurfaceCache, but also clamping the size consistently to a configured
maximum. This will cause us to upscale rasterized SVGs which is
undesirable visually but is a lower risk change that we can uplift to
beta than fixing the underlying performance issue.
Differential Revision: https://phabricator.services.mozilla.com/D27159
We were marking them used even if only a decode was requested.
This can cause us to hold extra decoded copies of the image around because we have a tendency to request decode at the intrinsic size.
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.
There is one main difference between raster images and vector images
with respect to factor of 2 scaling. Vector images may be scaled
infinitely and so we need to extend factor of 2 scaling to permit
growing instead of just shrinking. Also, we don't want to scale
infinitely, so we should configure a maximum size limit. This size limit
will apply even outside of factor of 2 scaling, and so the caller
(VectorImage) will need to be careful to take this into account.
There is one main difference between raster images and vector images
with respect to factor of 2 scaling. Vector images may be scaled
infinitely and so we need to extend factor of 2 scaling to permit
growing instead of just shrinking. Also, we don't want to scale
infinitely, so we should configure a maximum size limit. This size limit
will apply even outside of factor of 2 scaling, and so the caller
(VectorImage) will need to be careful to take this into account.
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
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.
Most cases where the pointer is stored into an already-declared variable can
trivially be changed to MakeNotNull<T*>, as the NotNull raw pointer will end
up in a smart pointer.
In RAII cases, the target type can be specified (e.g.:
`MakeNotNull<RefPtr<imgFrame>>)`), in which case the variable type may just be
`auto`, similar to the common use of MakeUnique.
Except when the target type is a base pointer, in which case it must be
specified in the declaration.
MozReview-Commit-ID: BYaSsvMhiDi
--HG--
extra : rebase_source : 8fe6f2aeaff5f515b7af2276c439004fa3a1f3ab
There are a number of operations with the surface cache which may result
in individual surfaces for a particular image cache to be removed. If an
image cache is emptied, and we are in factor of 2 mode, we should reset
it to the default mode, because we require at least one surface to be
available to determine the native/ideal size. Additionally, if the cache
is not locked, it should be removed entirely from the surface cache. We
handle this correctly in methods such as Lookup and LookupBestMatch, but
Prune and CollectSizeOfSurfaces can also cause this to happen, as
recently done in bug 1370412 and bug 1380649.
When we lookup a surface in the cache, we are careful to remove any
surfaces which were backed by volatile memory and got purged before we
could reacquire the buffer. We were not so careful in doing that when
generating memory reports. ISurfaceProvider::AddSizeOfExcludingThis will
cause us to acquire the buffer, and if it was purged, forget about its
purged status. Later when we performed a lookup, we would forget the
purged status, and assume we have the right data. This would appear as
completely transparent for BGRA surfaces, and completely black for BGRX
surfaces.
With this patch, we now properly remove purged surfaces instead of
including them in the report. This ensures that the cache state is
consistent. This also resolves memory reports of surfaces which reported
using no data -- they were purged when the report was generated.
Additionally, there was a bug in SurfaceCache::PruneImage where we did
not discard surfaces outside the module lock. Both PruneImage and
CollectSizeOfSurfaces now free any discarded surfaces outside the lock.
When we lookup a surface in the cache, we are careful to remove any
surfaces which were backed by volatile memory and got purged before we
could reacquire the buffer. We were not so careful in doing that when
generating memory reports. ISurfaceProvider::AddSizeOfExcludingThis will
cause us to acquire the buffer, and if it was purged, forget about its
purged status. Later when we performed a lookup, we would forget the
purged status, and assume we have the right data. This would appear as
completely transparent for BGRA surfaces, and completely black for BGRX
surfaces.
With this patch, we now properly remove purged surfaces instead of
including them in the report. This ensures that the cache state is
consistent. This also resolves memory reports of surfaces which reported
using no data -- they were purged when the report was generated.
Additionally, there was a bug in SurfaceCache::PruneImage where we did
not discard surfaces outside the module lock. Both PruneImage and
CollectSizeOfSurfaces now free any discarded surfaces outside the lock.
When the surface cache starts tracking an unlocked surface, it must
insert it into the expiration tracker, so that it can be freed later if
it is remains unused. ExpirationTrackerImpl::AddObjectLocked can fail
due to out-of-memory conditions or during shutdown, which we previously
ignored, and could leave us in a state where we think the surface is in
the tracker but is not. When we later try to mark the surface as used in
the tracker, it will hit a release assert because it doesn't exist. Now
we handle the insertion failure by discarding the surface. Marking the
surface as used can itself encounter a similar issue, and we handle it
the same way.
MozReview-Commit-ID: Kv6l0znnG48
An ImageSurfaceCache cannot enter factor-of-2 mode without a minimum
number of surfaces being present in its cache. However those surfaces
can be purged from the cache through various means (expire due to being
disuse, volatile buffers purged, etc). Also, it is entirely possible
that all the surfaces get purged, but the cache itself remains. Since
factor-of-2 mode requires at least one surface (to get the owning image
and its native size), we need to handle the case when the cache is
emptied appropriately. As such, we now reset the factor-of-2 mode (and
its pruned state) to the default (false) if we transition from non-empty
to empty.
MozReview-Commit-ID: EVaEqW59Asv
Timothy Nikkel
championshuttler
Brindusan Cristian
Bogdan Tara
Nicholas Nethercote
Jean-Yves Avenard
Gurzau Raul
Andrew Osmond
Ryan Hunt
Sylvestre Ledru
Andrew McCreight
Narcis Beleuzu
Andreea Pavel
Brian Hackett
Emilio Cobos Álvarez
Gerald Squelart
Sebastian Hengst