0

Optimize ClientServiceMap.

This CL has the following small optimizations:

- Inlines GetServiceID
- Adds HasClientID so GetServiceID can remove some ifs
- Refactors GetTextureServiceID for better inlining

Improves the score of the TextureDraw decoder perftest by 3-6%.

TextureDraw scores: (gl-null, passthrough, w/ WIP patches):

after CL:  12227 (stddev 65)
before CL: 12993 (stddev 68)
w/ lambda: 12351 (stddev 58)

Bug: angleproject:2763
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
Change-Id: I49968d68f4b6d8a64be3c937f201d69d55de17ad
Reviewed-on: https://chromium-review.googlesource.com/c/1265089
Reviewed-by: Antoine Labour <piman@chromium.org>
Commit-Queue: Jamie Madill <jmadill@chromium.org>
Cr-Commit-Position: refs/heads/master@{#597676}
This commit is contained in:
Jamie Madill
2018-10-08 20:49:54 +00:00
committed by Commit Bot
parent f289edff3c
commit aa19f5dba9
4 changed files with 66 additions and 48 deletions

@ -63,33 +63,35 @@ class ClientServiceMap {
client_to_service_map_.clear();
}
bool GetServiceID(ClientType client_id, ServiceType* service_id) const {
if (client_id < kMaxFlatArraySize) {
if (client_id < client_to_service_array_.size() &&
client_to_service_array_[client_id] != invalid_service_id()) {
if (service_id) {
*service_id = client_to_service_array_[client_id];
}
return true;
}
} else {
auto iter = client_to_service_map_.find(client_id);
if (iter != client_to_service_map_.end()) {
if (service_id) {
*service_id = iter->second;
}
return true;
}
ALWAYS_INLINE bool GetServiceID(ClientType client_id,
ServiceType* service_id) const {
DCHECK(service_id);
if (client_id >= kMaxFlatArraySize) {
return GetServiceIDFromMap(client_id, service_id);
}
if (client_id < client_to_service_array_.size() &&
client_to_service_array_[client_id] != invalid_service_id()) {
*service_id = client_to_service_array_[client_id];
return true;
}
if (client_id == 0) {
if (service_id) {
*service_id = 0;
}
*service_id = 0;
return true;
}
return false;
}
ALWAYS_INLINE bool HasClientID(ClientType client_id) const {
if (client_id >= kMaxFlatArraySize) {
return GetServiceIDFromMap(client_id, nullptr);
}
return client_id == 0 ||
(client_id < client_to_service_array_.size() &&
client_to_service_array_[client_id] != invalid_service_id());
}
ServiceType GetServiceIDOrInvalid(ClientType client_id) {
ServiceType service_id;
if (GetServiceID(client_id, &service_id)) {
@ -118,10 +120,7 @@ class ClientServiceMap {
}
}
if (service_id == 0) {
if (client_id) {
*client_id = 0;
}
return true;
*client_id = 0;
}
return false;
}
@ -148,6 +147,19 @@ class ClientServiceMap {
static constexpr size_t kMaxFlatArraySize = 0x4000;
private:
bool GetServiceIDFromMap(ClientType client_id,
ServiceType* service_id) const {
DCHECK(client_id >= kMaxFlatArraySize);
auto iter = client_to_service_map_.find(client_id);
if (iter != client_to_service_map_.end()) {
if (service_id) {
*service_id = iter->second;
}
return true;
}
return false;
}
ServiceType invalid_service_id_;
std::vector<ServiceType> client_to_service_array_;
std::unordered_map<ClientType, ServiceType> client_to_service_map_;

@ -25,7 +25,7 @@ TEST(ClientServiceMap, BasicMapping) {
EXPECT_EQ(kServiceId, service_id);
// Check null is handled for GetServiceID
EXPECT_TRUE(map.GetServiceID(kClientId, nullptr));
EXPECT_TRUE(map.HasClientID(kClientId));
// Check service -> client ID lookup
MapDataType client_id = 0;
@ -90,19 +90,19 @@ TEST(ClientServiceMap, RemoveMapping) {
ClientServiceMapType map;
map.SetIDMapping(kClientId, kServiceId);
EXPECT_TRUE(map.GetServiceID(kClientId, nullptr));
EXPECT_TRUE(map.HasClientID(kClientId));
map.RemoveClientID(kClientId);
EXPECT_FALSE(map.GetServiceID(kClientId, nullptr));
EXPECT_FALSE(map.HasClientID(kClientId));
map.SetIDMapping(kClientId, kServiceId);
EXPECT_TRUE(map.GetServiceID(kClientId, nullptr));
EXPECT_TRUE(map.HasClientID(kClientId));
map.Clear();
EXPECT_FALSE(map.GetServiceID(kClientId, nullptr));
EXPECT_FALSE(map.HasClientID(kClientId));
map.SetIDMapping(kClientId, kLargeServiceId);
EXPECT_TRUE(map.GetServiceID(kClientId, nullptr));
EXPECT_TRUE(map.HasClientID(kClientId));
map.RemoveClientID(kClientId);
EXPECT_FALSE(map.GetServiceID(kClientId, nullptr));
EXPECT_FALSE(map.HasClientID(kClientId));
}
TEST(ClientServiceMap, ManyIDs) {

@ -143,13 +143,12 @@ void PassthroughResources::Destroy(gl::GLApi* api) {
bool have_context = !!api;
// Only delete textures that are not referenced by a TexturePassthrough
// object, they handle their own deletion once all references are lost
DeleteServiceObjects(
&texture_id_map, have_context,
[this, api](GLuint client_id, GLuint texture) {
if (!texture_object_map.GetServiceID(client_id, nullptr)) {
api->glDeleteTexturesFn(1, &texture);
}
});
DeleteServiceObjects(&texture_id_map, have_context,
[this, api](GLuint client_id, GLuint texture) {
if (!texture_object_map.HasClientID(client_id)) {
api->glDeleteTexturesFn(1, &texture);
}
});
DeleteServiceObjects(&buffer_id_map, have_context,
[api](GLuint client_id, GLuint buffer) {
api->glDeleteBuffersARBFn(1, &buffer);

@ -26,7 +26,7 @@ error::Error GenHelper(GLsizei n,
DCHECK(n >= 0);
std::vector<ClientType> client_ids_copy(client_ids, client_ids + n);
for (GLsizei ii = 0; ii < n; ++ii) {
if (id_map->GetServiceID(client_ids_copy[ii], nullptr)) {
if (id_map->HasClientID(client_ids_copy[ii])) {
return error::kInvalidArguments;
}
}
@ -47,7 +47,7 @@ template <typename ClientType, typename ServiceType, typename GenFunction>
error::Error CreateHelper(ClientType client_id,
ClientServiceMap<ClientType, ServiceType>* id_map,
GenFunction create_function) {
if (id_map->GetServiceID(client_id, nullptr)) {
if (id_map->HasClientID(client_id)) {
return error::kInvalidArguments;
}
ServiceType service_id = create_function();
@ -110,12 +110,19 @@ GLuint GetTextureServiceID(gl::GLApi* api,
GLuint client_id,
PassthroughResources* resources,
bool create_if_missing) {
return GetServiceID(client_id, &resources->texture_id_map, create_if_missing,
[api]() {
GLuint service_id = 0;
api->glGenTexturesFn(1, &service_id);
return service_id;
});
GLuint service_id = resources->texture_id_map.invalid_service_id();
if (resources->texture_id_map.GetServiceID(client_id, &service_id)) {
return service_id;
}
if (create_if_missing) {
GLuint service_id = 0;
api->glGenTexturesFn(1, &service_id);
resources->texture_id_map.SetIDMapping(client_id, service_id);
return service_id;
}
return resources->texture_id_map.invalid_service_id();
}
GLuint GetBufferServiceID(gl::GLApi* api,
@ -1091,7 +1098,7 @@ error::Error GLES2DecoderPassthroughImpl::DoEnableVertexAttribArray(
error::Error GLES2DecoderPassthroughImpl::DoFenceSync(GLenum condition,
GLbitfield flags,
GLuint client_id) {
if (resources_->sync_id_map.GetServiceID(client_id, nullptr)) {
if (resources_->sync_id_map.HasClientID(client_id)) {
return error::kInvalidArguments;
}
@ -4038,7 +4045,7 @@ error::Error GLES2DecoderPassthroughImpl::DoCreateAndConsumeTextureINTERNAL(
GLuint texture_client_id,
const volatile GLbyte* mailbox) {
if (!texture_client_id ||
resources_->texture_id_map.GetServiceID(texture_client_id, nullptr)) {
resources_->texture_id_map.HasClientID(texture_client_id)) {
return error::kInvalidArguments;
}