Skip to content

Commit

Permalink
Cache ImageContext in DisplayResourceProvider
Browse files Browse the repository at this point in the history 
This avoids flat_map look ups in multiple places and saves about 1% of
CPU in RendererPerfTest/1.TextureQuads5x5 microbenchmark.

ImageContext was split into 2 parts to help with layering:
- ExternalUseClient::ImageContext is used by display
- ImageContextImpl is used by display_embedder

Notes about thread safety were added to external_use_client.h
viz::ResourceMetadata is removed because it is no longer necessary.

Moving IsTextureResource to DCHECK removes unnecessary map look up on
release builds.

Bug: 974359
Change-Id: I84f8f57856b63bb2b371ac32349a3ba794b53717
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1713298
Commit-Queue: Jonathan Backer <backer@chromium.org>
Reviewed-by: kylechar <kylechar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#684975}
  • Loading branch information
Jonathan Backer authored and Commit Bot committed Aug 7, 2019
1 parent e6e62b7 commit ea00a40
Show file tree
Hide file tree
Showing 24 changed files with 838 additions and 769 deletions.
7 changes: 3 additions & 4 deletions components/viz/service/BUILD.gn
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ viz_component("service") {
"display/draw_polygon.h",
"display/dynamic_geometry_binding.cc",
"display/dynamic_geometry_binding.h",
"display/external_use_client.cc",
"display/external_use_client.h",
"display/frame_rate_decider.cc",
"display/frame_rate_decider.h",
Expand Down Expand Up @@ -75,8 +76,6 @@ viz_component("service") {
"display/renderer_utils.cc",
"display/renderer_utils.h",
"display/resource_fence.h",
"display/resource_metadata.cc",
"display/resource_metadata.h",
"display/scoped_gpu_memory_buffer_texture.cc",
"display/scoped_gpu_memory_buffer_texture.h",
"display/scoped_render_pass_texture.cc",
Expand Down Expand Up @@ -308,8 +307,8 @@ viz_component("service") {
# then this build target will no longer be needed.
viz_source_set("gpu_service_dependencies") {
sources = [
"display_embedder/image_context.cc",
"display_embedder/image_context.h",
"display_embedder/image_context_impl.cc",
"display_embedder/image_context_impl.h",
"display_embedder/skia_output_device.cc",
"display_embedder/skia_output_device.h",
"display_embedder/skia_output_device_buffer_queue.cc",
Expand Down
137 changes: 70 additions & 67 deletions components/viz/service/display/display_resource_provider.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@

#include "components/viz/service/display/display_resource_provider.h"

#include <algorithm>
#include <string>

#include "base/atomic_sequence_num.h"
#include "base/numerics/safe_math.h"
#include "base/stl_util.h"
Expand Down Expand Up @@ -535,66 +538,11 @@ void DisplayResourceProvider::UnlockForRead(ResourceId id) {
gl->EndSharedImageAccessDirectCHROMIUM(resource->gl_id);
}
resource->lock_for_read_count--;
TryReleaseResource(it);
}

ResourceMetadata DisplayResourceProvider::LockForExternalUse(ResourceId id) {
DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
auto it = resources_.find(id);
DCHECK(it != resources_.end());

ChildResource* resource = &it->second;
ResourceMetadata metadata;
// Make sure there is no outstanding LockForExternalUse without calling
// UnlockForExternalUse.
DCHECK(!resource->locked_for_external_use);
// TODO(penghuang): support software resource.
DCHECK(resource->is_gpu_resource_type());

metadata.resource_id = id;
metadata.mailbox_holder = resource->transferable.mailbox_holder;
metadata.size = resource->transferable.size;
metadata.resource_format = resource->transferable.format;
metadata.mailbox_holder.sync_token = resource->sync_token();
metadata.color_space = resource->transferable.color_space;
metadata.origin = kTopLeft_GrSurfaceOrigin;

resource->locked_for_external_use = true;

if (resource->transferable.read_lock_fences_enabled)
resource->read_lock_fence = current_read_lock_fence_;

return metadata;
}

void DisplayResourceProvider::UnlockForExternalUse(
ResourceId id,
const gpu::SyncToken& sync_token) {
DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
auto it = resources_.find(id);
DCHECK(it != resources_.end());
DCHECK(sync_token.verified_flush());

ChildResource* resource = &it->second;
DCHECK(resource->locked_for_external_use);
// TODO(penghuang): support software resource.
DCHECK(resource->is_gpu_resource_type());

// Update the resource sync token to |sync_token|. When the next frame is
// being composited, the DeclareUsedResourcesFromChild() will be called with
// resources belong to every child for the next frame. If the resource is not
// used by the next frame, the resource will be returned to a child which
// owns it with the |sync_token|. The child is responsible for issuing a
// WaitSyncToken GL command with the |sync_token| before reusing it.
resource->UpdateSyncToken(sync_token);
resource->locked_for_external_use = false;

TryReleaseResource(it);
TryReleaseResource(id, resource);
}

void DisplayResourceProvider::TryReleaseResource(ResourceMap::iterator it) {
ResourceId id = it->first;
ChildResource* resource = &it->second;
void DisplayResourceProvider::TryReleaseResource(ResourceId id,
ChildResource* resource) {
if (resource->marked_for_deletion && !resource->lock_for_read_count &&
!resource->locked_for_external_use) {
auto child_it = children_.find(resource->child_id);
Expand Down Expand Up @@ -677,6 +625,10 @@ void DisplayResourceProvider::DeleteAndReturnUnusedResourcesToChild(
std::vector<ReturnedResource*> need_synchronization_resources;
std::vector<GLbyte*> unverified_sync_tokens;

std::vector<std::unique_ptr<ExternalUseClient::ImageContext>>
image_contexts_to_return;
image_contexts_to_return.reserve(unused.size());

GLES2Interface* gl = ContextGL();
for (ResourceId local_id : unused) {
auto it = resources_.find(local_id);
Expand Down Expand Up @@ -712,6 +664,9 @@ void DisplayResourceProvider::DeleteAndReturnUnusedResourcesToChild(
is_lost = true;
}

if (resource.image_context)
image_contexts_to_return.emplace_back(std::move(resource.image_context));

if (resource.is_gpu_resource_type() &&
resource.filter != resource.transferable.filter) {
DCHECK(resource.transferable.mailbox_holder.texture_target);
Expand Down Expand Up @@ -766,8 +721,10 @@ void DisplayResourceProvider::DeleteAndReturnUnusedResourcesToChild(
for (ReturnedResource* returned : need_synchronization_resources)
returned->sync_token = new_sync_token;

if (external_use_client_ && !unused.empty())
external_use_client_->ReleaseCachedResources(unused);
if (external_use_client_ && !image_contexts_to_return.empty()) {
external_use_client_->ReleaseImageContexts(
std::move(image_contexts_to_return));
}

if (!to_return.empty())
child_info->return_callback.Run(to_return);
Expand Down Expand Up @@ -938,17 +895,63 @@ DisplayResourceProvider::LockSetForExternalUse::~LockSetForExternalUse() {
DCHECK(resources_.empty());
}

ResourceMetadata DisplayResourceProvider::LockSetForExternalUse::LockResource(
ResourceId id) {
DCHECK(!base::Contains(resources_, id));
resources_.push_back(id);
return resource_provider_->LockForExternalUse(id);
ExternalUseClient::ImageContext*
DisplayResourceProvider::LockSetForExternalUse::LockResource(
ResourceId id,
bool is_video_plane) {
auto it = resource_provider_->resources_.find(id);
DCHECK(it != resource_provider_->resources_.end());

ChildResource& resource = it->second;
DCHECK(resource.is_gpu_resource_type());

if (!resource.locked_for_external_use) {
DCHECK(!base::Contains(resources_, std::make_pair(id, &resource)));
resources_.emplace_back(id, &resource);

if (!resource.image_context) {
resource.image_context =
resource_provider_->external_use_client_->CreateImageContext(
resource.transferable.mailbox_holder, resource.transferable.size,
resource.transferable.format,
// The resource |color_space| is ignored by SkiaRenderer for video
// planes and usually cannot be converted to SkColorSpace.
is_video_plane
? nullptr
: resource.transferable.color_space.ToSkColorSpace());
}
resource.locked_for_external_use = true;

if (resource.transferable.read_lock_fences_enabled)
resource.read_lock_fence = resource_provider_->current_read_lock_fence_;
}

DCHECK(base::Contains(resources_, std::make_pair(id, &resource)));
return resource.image_context.get();
}

void DisplayResourceProvider::LockSetForExternalUse::UnlockResources(
const gpu::SyncToken& sync_token) {
for (const auto& id : resources_)
resource_provider_->UnlockForExternalUse(id, sync_token);
DCHECK(sync_token.verified_flush());
for (const auto& pair : resources_) {
auto id = pair.first;
auto* resource = pair.second;
DCHECK(resource->locked_for_external_use);

// TODO(penghuang): support software resource.
DCHECK(resource->is_gpu_resource_type());

// Update the resource sync token to |sync_token|. When the next frame is
// being composited, the DeclareUsedResourcesFromChild() will be called with
// resources belong to every child for the next frame. If the resource is
// not used by the next frame, the resource will be returned to a child
// which owns it with the |sync_token|. The child is responsible for issuing
// a WaitSyncToken GL command with the |sync_token| before reusing it.
resource->UpdateSyncToken(sync_token);
resource->locked_for_external_use = false;

resource_provider_->TryReleaseResource(id, resource);
}
resources_.clear();
}

Expand Down
39 changes: 22 additions & 17 deletions components/viz/service/display/display_resource_provider.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,11 @@
#define COMPONENTS_VIZ_SERVICE_DISPLAY_DISPLAY_RESOURCE_PROVIDER_H_

#include <stddef.h>

#include <map>
#include <memory>
#include <unordered_map>
#include <utility>
#include <vector>

#include "base/containers/flat_map.h"
Expand All @@ -23,7 +26,6 @@
#include "components/viz/service/display/external_use_client.h"
#include "components/viz/service/display/overlay_candidate.h"
#include "components/viz/service/display/resource_fence.h"
#include "components/viz/service/display/resource_metadata.h"
#include "components/viz/service/viz_service_export.h"
#include "gpu/command_buffer/common/sync_token.h"
#include "third_party/khronos/GLES2/gl2.h"
Expand Down Expand Up @@ -195,6 +197,11 @@ class VIZ_SERVICE_EXPORT DisplayResourceProvider
DISALLOW_COPY_AND_ASSIGN(ScopedReadLockSkImage);
};

private:
// Forward declared for LockSetForExternalUse below.
struct ChildResource;

public:
// Maintains set of resources locked for external use by SkiaRenderer.
class VIZ_SERVICE_EXPORT LockSetForExternalUse {
public:
Expand All @@ -205,17 +212,20 @@ class VIZ_SERVICE_EXPORT DisplayResourceProvider
ExternalUseClient* client);
~LockSetForExternalUse();

// Lock a resource for external use.
ResourceMetadata LockResource(ResourceId resource_id);
// Lock a resource for external use. The return value was created by
// |client| at some point in the past.
ExternalUseClient::ImageContext* LockResource(ResourceId resource_id,
bool is_video_plane = false);

// Unlock all locked resources with a |sync_token|.
// See UnlockForExternalUse for the detail. All resources must be unlocked
// before destroying this class.
// Unlock all locked resources with a |sync_token|. The |sync_token| should
// be waited on before reusing the resource's backing to ensure that any
// external use of it is completed. This |sync_token| should have been
// verified. All resources must be unlocked before destroying this class.
void UnlockResources(const gpu::SyncToken& sync_token);

private:
DisplayResourceProvider* const resource_provider_;
std::vector<ResourceId> resources_;
std::vector<std::pair<ResourceId, ChildResource*>> resources_;

DISALLOW_COPY_AND_ASSIGN(LockSetForExternalUse);
};
Expand Down Expand Up @@ -415,6 +425,10 @@ class VIZ_SERVICE_EXPORT DisplayResourceProvider
// texture.
scoped_refptr<ResourceFence> read_lock_fence;

// SkiaRenderer specific details about this resource. Added to ChildResource
// to avoid map lookups further down the pipeline.
std::unique_ptr<ExternalUseClient::ImageContext> image_context;

private:
// Tracks if a sync token needs to be waited on before using the resource.
SynchronizationState synchronization_state_;
Expand Down Expand Up @@ -448,16 +462,7 @@ class VIZ_SERVICE_EXPORT DisplayResourceProvider
const ChildResource* LockForRead(ResourceId id);
void UnlockForRead(ResourceId id);

// Lock a resource for external use.
ResourceMetadata LockForExternalUse(ResourceId id);

// Unlock a resource which locked by LockForExternalUse.
// The |sync_token| should be waited on before reusing the resouce's backing
// to ensure that any external use of it is completed. This |sync_token|
// should have been verified.
void UnlockForExternalUse(ResourceId id, const gpu::SyncToken& sync_token);

void TryReleaseResource(ResourceMap::iterator it);
void TryReleaseResource(ResourceId id, ChildResource* resource);
// Binds the given GL resource to a texture target for sampling using the
// specified filter for both minification and magnification. Returns the
// texture target used. The resource must be locked for reading.
Expand Down

0 comments on commit ea00a40

Please sign in to comment.