From 2fb97a8843dfddcd56b7b73bd137674e0fe93cd9 Mon Sep 17 00:00:00 2001 From: "backer@chromium.org" Date: Thu, 31 Jan 2013 18:36:09 +0000 Subject: Merge 179016 > Aura: Throttle --ui-prioritize-in-gpu-process > > Without this change, --ui-prioritize-in-gpu-process will preempt other clients > to service the browser UI whenever the browser UI wants to draw. This allows > the browser to starve other clients of the GPU process. > > With this change, the browser has to wait 16 ms (a full frame) before > preempting other clients. This moderately affects interactivity (tested by > dragging a window on x86-alex while running > https://www.khronos.org/registry/webgl/sdk/demos/google/san-angeles/index.html) > but improves fairness and prevents starvation. > > R=piman > BUG=171135,chrome-os-partner:14133,148382,chromium-os:37557 > > > Review URL: https://chromiumcodereview.appspot.com/12040077 TBR=backer@chromium.org Review URL: https://codereview.chromium.org/12096092 git-svn-id: svn://svn.chromium.org/chrome/branches/1364/src@179910 0039d316-1c4b-4281-b951-d872f2087c98 --- content/common/gpu/gpu_channel.cc | 250 ++++++++++++--------- content/common/gpu/gpu_channel.h | 33 ++- content/common/gpu/gpu_command_buffer_stub.cc | 12 +- content/common/gpu/gpu_command_buffer_stub.h | 4 +- content/common/gpu/image_transport_surface.cc | 6 +- content/common/gpu/image_transport_surface.h | 6 +- .../common/gpu/texture_image_transport_surface.cc | 2 +- 7 files changed, 182 insertions(+), 131 deletions(-) (limited to 'content') diff --git a/content/common/gpu/gpu_channel.cc b/content/common/gpu/gpu_channel.cc index 64e6a19..217d9ef 100644 --- a/content/common/gpu/gpu_channel.cc +++ b/content/common/gpu/gpu_channel.cc @@ -15,6 +15,7 @@ #include "base/process_util.h" #include "base/rand_util.h" #include "base/string_util.h" +#include "base/timer.h" #include "content/common/child_process.h" #include "content/common/gpu/gpu_channel_manager.h" #include "content/common/gpu/gpu_messages.h" @@ -40,6 +41,87 @@ namespace content { namespace { + +// How long to wait for an IPC on this channel to be processed before +// signaling the need for a preemption. +// TODO(backer): This should really be the time between vblank on +// platforms that provide this information. +const int64 kPreemptPeriodMs = 16; + +// Generates mailbox names for clients of the GPU process on the IO thread. +class MailboxMessageFilter : public IPC::ChannelProxy::MessageFilter { + public: + explicit MailboxMessageFilter(const std::string& private_key) + : channel_(NULL), + hmac_(crypto::HMAC::SHA256) { + bool success = hmac_.Init(base::StringPiece(private_key)); + DCHECK(success); + } + + virtual void OnFilterAdded(IPC::Channel* channel) { + DCHECK(!channel_); + channel_ = channel; + } + + virtual void OnFilterRemoved() { + DCHECK(channel_); + channel_ = NULL; + } + + virtual bool OnMessageReceived(const IPC::Message& message) { + DCHECK(channel_); + + bool handled = true; + IPC_BEGIN_MESSAGE_MAP(MailboxMessageFilter, message) + IPC_MESSAGE_HANDLER(GpuChannelMsg_GenerateMailboxNames, + OnGenerateMailboxNames) + IPC_MESSAGE_HANDLER(GpuChannelMsg_GenerateMailboxNamesAsync, + OnGenerateMailboxNamesAsync) + IPC_MESSAGE_UNHANDLED(handled = false) + IPC_END_MESSAGE_MAP() + + return handled; + } + + bool Send(IPC::Message* message) { + return channel_->Send(message); + } + + private: + ~MailboxMessageFilter() { + } + + // Message handlers. + void OnGenerateMailboxNames(unsigned num, std::vector* result) { + TRACE_EVENT1("gpu", "OnGenerateMailboxNames", "num", num); + + result->resize(num); + + for (unsigned i = 0; i < num; ++i) { + char name[GL_MAILBOX_SIZE_CHROMIUM]; + base::RandBytes(name, sizeof(name) / 2); + + bool success = hmac_.Sign( + base::StringPiece(name, sizeof(name) / 2), + reinterpret_cast(name) + sizeof(name) / 2, + sizeof(name) / 2); + DCHECK(success); + + (*result)[i].assign(name, sizeof(name)); + } + } + + void OnGenerateMailboxNamesAsync(unsigned num) { + std::vector names; + OnGenerateMailboxNames(num, &names); + Send(new GpuChannelMsg_GenerateMailboxNamesReply(names)); + } + + IPC::Channel* channel_; + crypto::HMAC hmac_; +}; +} // anonymous namespace + // This filter does two things: // - it counts the number of messages coming in on the channel // - it handles the GpuCommandBufferMsg_InsertSyncPoint message on the IO @@ -52,12 +134,13 @@ class SyncPointMessageFilter : public IPC::ChannelProxy::MessageFilter { SyncPointMessageFilter(base::WeakPtr* gpu_channel, scoped_refptr sync_point_manager, scoped_refptr message_loop, - scoped_refptr - unprocessed_messages) + scoped_refptr processing_stalled, + base::AtomicRefCount* unprocessed_messages) : gpu_channel_(gpu_channel), channel_(NULL), sync_point_manager_(sync_point_manager), message_loop_(message_loop), + processing_stalled_(processing_stalled), unprocessed_messages_(unprocessed_messages) { } @@ -73,7 +156,11 @@ class SyncPointMessageFilter : public IPC::ChannelProxy::MessageFilter { virtual bool OnMessageReceived(const IPC::Message& message) { DCHECK(channel_); - unprocessed_messages_->IncCount(); + base::AtomicRefCountInc(unprocessed_messages_.get()); + if (!timer_.IsRunning()) + timer_.Start(FROM_HERE, + base::TimeDelta::FromMilliseconds(kPreemptPeriodMs), + this, &SyncPointMessageFilter::TriggerPreemption); if (message.type() == GpuCommandBufferMsg_InsertSyncPoint::ID) { uint32 sync_point = sync_point_manager_->GenerateSyncPoint(); IPC::Message* reply = IPC::SyncMessage::GenerateReply(&message); @@ -85,18 +172,29 @@ class SyncPointMessageFilter : public IPC::ChannelProxy::MessageFilter { sync_point_manager_, message.routing_id(), sync_point, - unprocessed_messages_)); + unprocessed_messages_.get())); return true; } else if (message.type() == GpuCommandBufferMsg_RetireSyncPoint::ID) { // This message should not be sent explicitly by the renderer. NOTREACHED(); - unprocessed_messages_->DecCount(); + base::AtomicRefCountDec(unprocessed_messages_.get()); return true; } else { return false; } } + void TriggerPreemption() { + if (!base::AtomicRefCountIsZero(unprocessed_messages_.get())) + processing_stalled_->Set(); + } + + void ReschedulePreemption() { + processing_stalled_->Reset(); + if (timer_.IsRunning()) + timer_.Reset(); + } + protected: virtual ~SyncPointMessageFilter() { message_loop_->PostTask(FROM_HERE, base::Bind( @@ -109,7 +207,7 @@ class SyncPointMessageFilter : public IPC::ChannelProxy::MessageFilter { scoped_refptr manager, int32 routing_id, uint32 sync_point, - scoped_refptr unprocessed_messages_) { + base::AtomicRefCount* unprocessed_messages) { // This function must ensure that the sync point will be retired. Normally // we'll find the stub based on the routing ID, and associate the sync point // with it, but if that fails for any reason (channel or stub already @@ -124,7 +222,7 @@ class SyncPointMessageFilter : public IPC::ChannelProxy::MessageFilter { gpu_channel->get()->OnMessageReceived(message); return; } else { - unprocessed_messages_->DecCount(); + base::AtomicRefCountDec(unprocessed_messages); } } manager->RetireSyncPoint(sync_point); @@ -142,83 +240,11 @@ class SyncPointMessageFilter : public IPC::ChannelProxy::MessageFilter { IPC::Channel* channel_; scoped_refptr sync_point_manager_; scoped_refptr message_loop_; - scoped_refptr unprocessed_messages_; + scoped_refptr processing_stalled_; + scoped_ptr unprocessed_messages_; + base::OneShotTimer timer_; }; -// Generates mailbox names for clients of the GPU process on the IO thread. -class MailboxMessageFilter : public IPC::ChannelProxy::MessageFilter { - public: - explicit MailboxMessageFilter(const std::string& private_key) - : channel_(NULL), - hmac_(crypto::HMAC::SHA256) { - bool success = hmac_.Init(base::StringPiece(private_key)); - DCHECK(success); - } - - virtual void OnFilterAdded(IPC::Channel* channel) { - DCHECK(!channel_); - channel_ = channel; - } - - virtual void OnFilterRemoved() { - DCHECK(channel_); - channel_ = NULL; - } - - virtual bool OnMessageReceived(const IPC::Message& message) { - DCHECK(channel_); - - bool handled = true; - IPC_BEGIN_MESSAGE_MAP(MailboxMessageFilter, message) - IPC_MESSAGE_HANDLER(GpuChannelMsg_GenerateMailboxNames, - OnGenerateMailboxNames) - IPC_MESSAGE_HANDLER(GpuChannelMsg_GenerateMailboxNamesAsync, - OnGenerateMailboxNamesAsync) - IPC_MESSAGE_UNHANDLED(handled = false) - IPC_END_MESSAGE_MAP() - - return handled; - } - - bool Send(IPC::Message* message) { - return channel_->Send(message); - } - - private: - ~MailboxMessageFilter() { - } - - // Message handlers. - void OnGenerateMailboxNames(unsigned num, std::vector* result) { - TRACE_EVENT1("gpu", "OnGenerateMailboxNames", "num", num); - - result->resize(num); - - for (unsigned i = 0; i < num; ++i) { - char name[GL_MAILBOX_SIZE_CHROMIUM]; - base::RandBytes(name, sizeof(name) / 2); - - bool success = hmac_.Sign( - base::StringPiece(name, sizeof(name) / 2), - reinterpret_cast(name) + sizeof(name) / 2, - sizeof(name) / 2); - DCHECK(success); - - (*result)[i].assign(name, sizeof(name)); - } - } - - void OnGenerateMailboxNamesAsync(unsigned num) { - std::vector names; - OnGenerateMailboxNames(num, &names); - Send(new GpuChannelMsg_GenerateMailboxNamesReply(names)); - } - - IPC::Channel* channel_; - crypto::HMAC hmac_; -}; -} // anonymous namespace - GpuChannel::GpuChannel(GpuChannelManager* gpu_channel_manager, GpuWatchdog* watchdog, gfx::GLShareGroup* share_group, @@ -226,7 +252,8 @@ GpuChannel::GpuChannel(GpuChannelManager* gpu_channel_manager, int client_id, bool software) : gpu_channel_manager_(gpu_channel_manager), - unprocessed_messages_(new gpu::RefCountedCounter), + unprocessed_messages_(NULL), + processing_stalled_(new gpu::PreemptionFlag), client_id_(client_id), share_group_(share_group ? share_group : new gfx::GLShareGroup), mailbox_manager_(mailbox ? mailbox : new gpu::gles2::MailboxManager), @@ -266,12 +293,15 @@ bool GpuChannel::Init(base::MessageLoopProxy* io_message_loop, base::WeakPtr* weak_ptr(new base::WeakPtr( weak_factory_.GetWeakPtr())); - scoped_refptr filter(new SyncPointMessageFilter( + unprocessed_messages_ = new base::AtomicRefCount(0); + filter_ = new SyncPointMessageFilter( weak_ptr, gpu_channel_manager_->sync_point_manager(), base::MessageLoopProxy::current(), - unprocessed_messages_)); - channel_->AddFilter(filter); + processing_stalled_, + unprocessed_messages_); + io_message_loop_ = io_message_loop; + channel_->AddFilter(filter_); channel_->AddFilter( new MailboxMessageFilter(mailbox_manager_->private_key())); @@ -294,9 +324,7 @@ int GpuChannel::TakeRendererFileDescriptor() { #endif // defined(OS_POSIX) bool GpuChannel::OnMessageReceived(const IPC::Message& message) { - // Drop the count that was incremented on the IO thread because we are - // about to process that message. - unprocessed_messages_->DecCount(); + bool message_processed = true; if (log_messages_) { DVLOG(1) << "received message @" << &message << " on channel @" << this << " with type " << message.type(); @@ -323,18 +351,21 @@ bool GpuChannel::OnMessageReceived(const IPC::Message& message) { } deferred_messages_.insert(point, new IPC::Message(message)); - unprocessed_messages_->IncCount(); + message_processed = false; } else { // Move GetStateFast commands to the head of the queue, so the renderer // doesn't have to wait any longer than necessary. deferred_messages_.push_front(new IPC::Message(message)); - unprocessed_messages_->IncCount(); + message_processed = false; } } else { deferred_messages_.push_back(new IPC::Message(message)); - unprocessed_messages_->IncCount(); + message_processed = false; } + if (message_processed) + MessageProcessed(); + OnScheduled(); return true; @@ -365,7 +396,7 @@ void GpuChannel::RequeueMessage() { DCHECK(currently_processing_message_); deferred_messages_.push_front( new IPC::Message(*currently_processing_message_)); - unprocessed_messages_->IncCount(); + base::AtomicRefCountInc(unprocessed_messages_); currently_processing_message_ = NULL; } @@ -416,8 +447,8 @@ void GpuChannel::CreateViewCommandBuffer( watchdog_, software_, init_params.active_url)); - if (preempt_by_counter_.get()) - stub->SetPreemptByCounter(preempt_by_counter_); + if (preemption_flag_.get()) + stub->SetPreemptByFlag(preemption_flag_); router_.AddRoute(*route_id, stub.get()); stubs_.AddWithID(stub.release(), *route_id); #endif // ENABLE_GPU @@ -482,18 +513,18 @@ void GpuChannel::RemoveRoute(int32 route_id) { router_.RemoveRoute(route_id); } -void GpuChannel::SetPreemptByCounter( - scoped_refptr preempt_by_counter) { - preempt_by_counter_ = preempt_by_counter; +void GpuChannel::SetPreemptByFlag( + scoped_refptr preemption_flag) { + preemption_flag_ = preemption_flag; for (StubMap::Iterator it(&stubs_); !it.IsAtEnd(); it.Advance()) { - it.GetCurrentValue()->SetPreemptByCounter(preempt_by_counter_); + it.GetCurrentValue()->SetPreemptByFlag(preemption_flag_); } } GpuChannel::~GpuChannel() { - unprocessed_messages_->Reset(); + processing_stalled_->Reset(); } void GpuChannel::OnDestroy() { @@ -536,7 +567,7 @@ void GpuChannel::HandleMessage() { if (m->type() == GpuCommandBufferMsg_Echo::ID) { stub->DelayEcho(m); deferred_messages_.pop_front(); - unprocessed_messages_->DecCount(); + MessageProcessed(); if (!deferred_messages_.empty()) OnScheduled(); } @@ -545,7 +576,7 @@ void GpuChannel::HandleMessage() { scoped_ptr message(m); deferred_messages_.pop_front(); - unprocessed_messages_->DecCount(); + bool message_processed = true; processed_get_state_fast_ = (message->type() == GpuCommandBufferMsg_GetStateFast::ID); @@ -570,10 +601,12 @@ void GpuChannel::HandleMessage() { if (stub->HasUnprocessedCommands()) { deferred_messages_.push_front(new GpuCommandBufferMsg_Rescheduled( stub->route_id())); - unprocessed_messages_->IncCount(); + message_processed = false; } } } + if (message_processed) + MessageProcessed(); } if (!deferred_messages_.empty()) { @@ -609,8 +642,8 @@ void GpuChannel::OnCreateOffscreenCommandBuffer( 0, watchdog_, software_, init_params.active_url)); - if (preempt_by_counter_.get()) - stub->SetPreemptByCounter(preempt_by_counter_); + if (preemption_flag_.get()) + stub->SetPreemptByFlag(preemption_flag_); router_.AddRoute(route_id, stub.get()); stubs_.AddWithID(stub.release(), route_id); TRACE_EVENT1("gpu", "GpuChannel::OnCreateOffscreenCommandBuffer", @@ -692,4 +725,11 @@ void GpuChannel::OnCollectRenderingStatsForSurface( Send(reply_message); } +void GpuChannel::MessageProcessed() { + base::AtomicRefCountDec(unprocessed_messages_); + io_message_loop_->PostTask( + FROM_HERE, + base::Bind(&SyncPointMessageFilter::ReschedulePreemption, filter_)); +} + } // namespace content diff --git a/content/common/gpu/gpu_channel.h b/content/common/gpu/gpu_channel.h index 77d3edc..d42ad2e 100644 --- a/content/common/gpu/gpu_channel.h +++ b/content/common/gpu/gpu_channel.h @@ -36,7 +36,7 @@ class WaitableEvent; } namespace gpu { -struct RefCountedCounter; +class PreemptionFlag; namespace gles2 { class ImageManager; } @@ -51,6 +51,7 @@ class StreamTextureManagerAndroid; namespace content { class GpuChannelManager; class GpuWatchdog; +class SyncPointMessageFilter; // Encapsulates an IPC channel between the GPU process and one renderer // process. On the renderer side there's a corresponding GpuChannelHost. @@ -129,14 +130,14 @@ class GpuChannel : public IPC::Listener, void AddRoute(int32 route_id, IPC::Listener* listener); void RemoveRoute(int32 route_id); - gpu::RefCountedCounter* MessagesPendingCount() { - return unprocessed_messages_.get(); + gpu::PreemptionFlag* GetPreemptionFlag() { + return processing_stalled_.get(); } - // If preempt_by_counter->count is non-zero, any stub on this channel + // If |preemption_flag->IsSet()|, any stub on this channel // should stop issuing GL commands. Setting this to NULL stops deferral. - void SetPreemptByCounter( - scoped_refptr preempt_by_counter); + void SetPreemptByFlag( + scoped_refptr preemption_flag); #if defined(OS_ANDROID) StreamTextureManagerAndroid* stream_texture_manager() { @@ -180,6 +181,9 @@ class GpuChannel : public IPC::Listener, void OnCollectRenderingStatsForSurface( int32 surface_id, IPC::Message* reply_message); + // Decrement the count of unhandled IPC messages and defer preemption. + void MessageProcessed(); + // The lifetime of objects of this class is managed by a GpuChannelManager. // The GpuChannelManager destroy all the GpuChannels that they own when they // are destroyed. So a raw pointer is safe. @@ -187,13 +191,17 @@ class GpuChannel : public IPC::Listener, scoped_ptr channel_; - // Number of routed messages for pending processing on a stub. - scoped_refptr unprocessed_messages_; + // Pointer to number of routed messages that are pending processing on a + // stub. The lifetime is properly managed because we pass ownership to a + // SyncPointMessageFilter, which we hold a reference to. + base::AtomicRefCount* unprocessed_messages_; + + // Whether the processing of IPCs on this channel is stalled. + scoped_refptr processing_stalled_; // If non-NULL, all stubs on this channel should stop processing GL - // commands (via their GpuScheduler) when preempt_by_counter_->count - // is non-zero. - scoped_refptr preempt_by_counter_; + // commands (via their GpuScheduler) when preemption_flag_->IsSet() + scoped_refptr preemption_flag_; std::deque deferred_messages_; @@ -232,6 +240,9 @@ class GpuChannel : public IPC::Listener, base::WeakPtrFactory weak_factory_; + scoped_refptr filter_; + scoped_refptr io_message_loop_; + DISALLOW_COPY_AND_ASSIGN(GpuChannel); }; diff --git a/content/common/gpu/gpu_command_buffer_stub.cc b/content/common/gpu/gpu_command_buffer_stub.cc index e6e1e78..0edbf49 100644 --- a/content/common/gpu/gpu_command_buffer_stub.cc +++ b/content/common/gpu/gpu_command_buffer_stub.cc @@ -372,8 +372,8 @@ void GpuCommandBufferStub::OnInitialize( scheduler_.reset(new gpu::GpuScheduler(command_buffer_.get(), decoder_.get(), decoder_.get())); - if (preempt_by_counter_.get()) - scheduler_->SetPreemptByCounter(preempt_by_counter_); + if (preemption_flag_.get()) + scheduler_->SetPreemptByFlag(preemption_flag_); decoder_->set_engine(scheduler_.get()); @@ -883,11 +883,11 @@ void GpuCommandBufferStub::RemoveDestructionObserver( destruction_observers_.RemoveObserver(observer); } -void GpuCommandBufferStub::SetPreemptByCounter( - scoped_refptr counter) { - preempt_by_counter_ = counter; +void GpuCommandBufferStub::SetPreemptByFlag( + scoped_refptr flag) { + preemption_flag_ = flag; if (scheduler_.get()) - scheduler_->SetPreemptByCounter(preempt_by_counter_); + scheduler_->SetPreemptByFlag(preemption_flag_); } bool GpuCommandBufferStub::GetTotalGpuMemory(size_t* bytes) { diff --git a/content/common/gpu/gpu_command_buffer_stub.h b/content/common/gpu/gpu_command_buffer_stub.h index 0519e8c..071b337 100644 --- a/content/common/gpu/gpu_command_buffer_stub.h +++ b/content/common/gpu/gpu_command_buffer_stub.h @@ -130,7 +130,7 @@ class GpuCommandBufferStub // retire all sync points that haven't been previously retired. void AddSyncPoint(uint32 sync_point); - void SetPreemptByCounter(scoped_refptr counter); + void SetPreemptByFlag(scoped_refptr flag); private: GpuMemoryManager* GetMemoryManager(); @@ -244,7 +244,7 @@ class GpuCommandBufferStub bool delayed_work_scheduled_; - scoped_refptr preempt_by_counter_; + scoped_refptr preemption_flag_; GURL active_url_; size_t active_url_hash_; diff --git a/content/common/gpu/image_transport_surface.cc b/content/common/gpu/image_transport_surface.cc index 67120ef..f30a24e 100644 --- a/content/common/gpu/image_transport_surface.cc +++ b/content/common/gpu/image_transport_surface.cc @@ -168,9 +168,9 @@ void ImageTransportHelper::DeferToFence(base::Closure task) { scheduler->DeferToFence(task); } -void ImageTransportHelper::SetPreemptByCounter( - scoped_refptr preempt_by_counter) { - stub_->channel()->SetPreemptByCounter(preempt_by_counter); +void ImageTransportHelper::SetPreemptByFlag( + scoped_refptr preemption_flag) { + stub_->channel()->SetPreemptByFlag(preemption_flag); } bool ImageTransportHelper::MakeCurrent() { diff --git a/content/common/gpu/image_transport_surface.h b/content/common/gpu/image_transport_surface.h index b1f0f5b..017899b4 100644 --- a/content/common/gpu/image_transport_surface.h +++ b/content/common/gpu/image_transport_surface.h @@ -34,7 +34,7 @@ class GLSurface; namespace gpu { class GpuScheduler; -struct RefCountedCounter; +class PreemptionFlag; namespace gles2 { class GLES2Decoder; } @@ -124,8 +124,8 @@ class ImageTransportHelper void DeferToFence(base::Closure task); - void SetPreemptByCounter( - scoped_refptr preempt_by_counter); + void SetPreemptByFlag( + scoped_refptr preemption_flag); // Make the surface's context current. bool MakeCurrent(); diff --git a/content/common/gpu/texture_image_transport_surface.cc b/content/common/gpu/texture_image_transport_surface.cc index f72d0ec..829a276 100644 --- a/content/common/gpu/texture_image_transport_surface.cc +++ b/content/common/gpu/texture_image_transport_surface.cc @@ -74,7 +74,7 @@ bool TextureImageTransportSurface::Initialize() { if (parent_channel) { const CommandLine* command_line = CommandLine::ForCurrentProcess(); if (command_line->HasSwitch(switches::kUIPrioritizeInGpuProcess)) - helper_->SetPreemptByCounter(parent_channel->MessagesPendingCount()); + helper_->SetPreemptByFlag(parent_channel->GetPreemptionFlag()); } return true; -- cgit v1.1