Commit 17a922fb authored by erikchen's avatar erikchen Committed by Commit Bot

Implement native event observer for linux.

This CL uses ui::PlatformEventSource observer API to hook into pre and post
native event dispatch on Linux.

Timestamps for X11 events are in X11 server time. The implementation makes a
round-trip to the X11 server to obtain a delta, and then uses that delta with
base::TimeTicks::Now() to convert to a base::TimeTicks.

The overhead from the rount-trip seems reasonable. 99th percentile X11 RTT is
3ms. 99.9th percentile is 16ms. 99.99th percentile is 40ms. These include the
time for syscalls to base::TimeTicks::Now().

To further reduce overhead, this CL changes the UMA metric to only be computed
with 1/1000 frequency, to avoid two calls to base::TimeTicks::Now().

Note: The implementation of native event observer will be turned on via Finch
experiment to measure overhead.

Change-Id: I786537f093e862a7a19630c9150606a2eb8267d9
Bug: 859155
Reviewed-on: https://chromium-review.googlesource.com/1162658Reviewed-by: default avatarTimothy Dresser <tdresser@chromium.org>
Reviewed-by: default avatarAvi Drissman <avi@chromium.org>
Reviewed-by: default avatarAlexander Timin <altimin@chromium.org>
Reviewed-by: default avatarThomas Anderson <thomasanderson@chromium.org>
Commit-Queue: Erik Chen <erikchen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#581304}
parent a31e844b
...@@ -2091,7 +2091,10 @@ jumbo_source_set("browser") { ...@@ -2091,7 +2091,10 @@ jumbo_source_set("browser") {
if (use_x11) { if (use_x11) {
configs += [ "//build/config/linux:x11" ] configs += [ "//build/config/linux:x11" ]
deps += [ "//ui/gfx/x" ] deps += [
"//ui/events/platform/x11",
"//ui/gfx/x",
]
} }
if (use_pangocairo) { if (use_pangocairo) {
......
...@@ -4,6 +4,18 @@ ...@@ -4,6 +4,18 @@
#include "content/browser/scheduler/responsiveness/native_event_observer.h" #include "content/browser/scheduler/responsiveness/native_event_observer.h"
#include "ui/events/platform/platform_event_source.h"
#if defined(OS_LINUX)
#if defined(USE_X11)
#include "ui/events/platform/x11/x11_event_source.h" // nogncheck
#elif defined(USE_OZONE)
#include "ui/events/event.h"
#endif
#include "ui/events/platform_event.h"
#endif
namespace content { namespace content {
namespace responsiveness { namespace responsiveness {
...@@ -20,11 +32,54 @@ NativeEventObserver::~NativeEventObserver() { ...@@ -20,11 +32,54 @@ NativeEventObserver::~NativeEventObserver() {
} }
#if !defined(OS_MACOSX) #if !defined(OS_MACOSX)
// TODO(erikchen): Implement this for non-macOS platforms. #if defined(OS_LINUX)
void NativeEventObserver::RegisterObserver() {
ui::PlatformEventSource::GetInstance()->AddPlatformEventObserver(this);
}
void NativeEventObserver::DeregisterObserver() {
ui::PlatformEventSource::GetInstance()->RemovePlatformEventObserver(this);
}
void NativeEventObserver::WillProcessEvent(const ui::PlatformEvent& event) {
will_run_event_callback_.Run(&event);
}
void NativeEventObserver::DidProcessEvent(const ui::PlatformEvent& event) {
#if defined(USE_OZONE)
did_run_event_callback_.Run(&event, event->time_stamp());
#elif defined(USE_X11)
// X11 uses a uint32_t on the wire protocol. Xlib casts this to an unsigned
// long by prepending with 0s. We cast back to a uint32_t so that subtraction
// works properly when the timestamp overflows back to 0.
uint32_t event_server_time_ms =
static_cast<uint32_t>(ui::X11EventSource::GetInstance()->GetTimestamp());
uint32_t current_server_time_ms = static_cast<uint32_t>(
ui::X11EventSource::GetInstance()->GetCurrentServerTime());
int32_t delta_ms = current_server_time_ms - event_server_time_ms;
// If for some reason server time is not monotonically increasing, ignore it.
if (delta_ms < 0)
delta_ms = 0;
// Ignore pathologically long deltas. Server is probably having issues.
base::TimeDelta delta = base::TimeDelta::FromMilliseconds(delta_ms);
base::TimeDelta long_duration = base::TimeDelta::FromSeconds(30);
if (delta > long_duration)
delta = base::TimeDelta();
did_run_event_callback_.Run(&event, base::TimeTicks::Now() - delta);
#else
#error
#endif
}
#else // defined(OS_LINUX)
// TODO(erikchen): Implement this for non-macOS, non-Linux platforms.
// https://crbug.com/859155. // https://crbug.com/859155.
void NativeEventObserver::RegisterObserver() {} void NativeEventObserver::RegisterObserver() {}
void NativeEventObserver::DeregisterObserver() {} void NativeEventObserver::DeregisterObserver() {}
#endif #endif // defined(OS_LINUX)
#endif // !defined(OS_MACOSX)
} // namespace responsiveness } // namespace responsiveness
} // namespace content } // namespace content
...@@ -14,6 +14,10 @@ ...@@ -14,6 +14,10 @@
#include "content/public/browser/native_event_processor_observer_mac.h" #include "content/public/browser/native_event_processor_observer_mac.h"
#endif #endif
#if defined(OS_LINUX)
#include "ui/events/platform/platform_event_observer.h"
#endif
namespace content { namespace content {
namespace responsiveness { namespace responsiveness {
...@@ -31,6 +35,8 @@ namespace responsiveness { ...@@ -31,6 +35,8 @@ namespace responsiveness {
class CONTENT_EXPORT NativeEventObserver class CONTENT_EXPORT NativeEventObserver
#if defined(OS_MACOSX) #if defined(OS_MACOSX)
: public NativeEventProcessorObserver : public NativeEventProcessorObserver
#elif defined(OS_LINUX)
: public ui::PlatformEventObserver
#endif #endif
{ {
public: public:
...@@ -46,7 +52,12 @@ class CONTENT_EXPORT NativeEventObserver ...@@ -46,7 +52,12 @@ class CONTENT_EXPORT NativeEventObserver
// processor. The destructor will unregister the object. // processor. The destructor will unregister the object.
NativeEventObserver(WillRunEventCallback will_run_event_callback, NativeEventObserver(WillRunEventCallback will_run_event_callback,
DidRunEventCallback did_run_event_callback); DidRunEventCallback did_run_event_callback);
#if defined(OS_LINUX)
~NativeEventObserver() override;
#else
virtual ~NativeEventObserver(); virtual ~NativeEventObserver();
#endif
protected: protected:
#if defined(OS_MACOSX) #if defined(OS_MACOSX)
...@@ -55,6 +66,11 @@ class CONTENT_EXPORT NativeEventObserver ...@@ -55,6 +66,11 @@ class CONTENT_EXPORT NativeEventObserver
void WillRunNativeEvent(const void* opaque_identifier) override; void WillRunNativeEvent(const void* opaque_identifier) override;
void DidRunNativeEvent(const void* opaque_identifier, void DidRunNativeEvent(const void* opaque_identifier,
base::TimeTicks creation_time) override; base::TimeTicks creation_time) override;
#elif defined(OS_LINUX)
// PlatformEventObserver overrides:
// Exposed for tests.
void WillProcessEvent(const ui::PlatformEvent& event) override;
void DidProcessEvent(const ui::PlatformEvent& event) override;
#endif #endif
private: private:
......
...@@ -96,7 +96,8 @@ X11EventSource::X11EventSource(X11EventSourceDelegate* delegate, ...@@ -96,7 +96,8 @@ X11EventSource::X11EventSource(X11EventSourceDelegate* delegate,
display_(display), display_(display),
dispatching_event_(nullptr), dispatching_event_(nullptr),
dummy_initialized_(false), dummy_initialized_(false),
continue_stream_(true) { continue_stream_(true),
distribution_(0, 999) {
DCHECK(!instance_); DCHECK(!instance_);
instance_ = this; instance_ = this;
...@@ -156,7 +157,13 @@ Time X11EventSource::GetCurrentServerTime() { ...@@ -156,7 +157,13 @@ Time X11EventSource::GetCurrentServerTime() {
dummy_initialized_ = true; dummy_initialized_ = true;
} }
base::TimeTicks start = base::TimeTicks::Now(); // No need to measure Linux.X11.ServerRTT on every call.
// base::TimeTicks::Now() itself has non-trivial overhead.
bool measure_rtt = distribution_(generator_) == 0;
base::TimeTicks start;
if (measure_rtt)
start = base::TimeTicks::Now();
// Make a no-op property change on |dummy_window_|. // Make a no-op property change on |dummy_window_|.
XChangeProperty(display_, dummy_window_, dummy_atom_, XA_STRING, 8, XChangeProperty(display_, dummy_window_, dummy_atom_, XA_STRING, 8,
...@@ -167,9 +174,12 @@ Time X11EventSource::GetCurrentServerTime() { ...@@ -167,9 +174,12 @@ Time X11EventSource::GetCurrentServerTime() {
XIfEvent(display_, &event, IsPropertyNotifyForTimestamp, XIfEvent(display_, &event, IsPropertyNotifyForTimestamp,
reinterpret_cast<XPointer>(&dummy_window_)); reinterpret_cast<XPointer>(&dummy_window_));
UMA_HISTOGRAM_CUSTOM_COUNTS( if (measure_rtt) {
"Linux.X11.ServerRTT", (base::TimeTicks::Now() - start).InMicroseconds(), UMA_HISTOGRAM_CUSTOM_COUNTS(
1, base::TimeDelta::FromMilliseconds(50).InMicroseconds(), 50); "Linux.X11.ServerRTT",
(base::TimeTicks::Now() - start).InMicroseconds(), 1,
base::TimeDelta::FromMilliseconds(50).InMicroseconds(), 50);
}
return event.xproperty.time; return event.xproperty.time;
} }
......
...@@ -8,6 +8,7 @@ ...@@ -8,6 +8,7 @@
#include <stdint.h> #include <stdint.h>
#include <memory> #include <memory>
#include <random>
#include "base/macros.h" #include "base/macros.h"
#include "base/optional.h" #include "base/optional.h"
...@@ -79,6 +80,10 @@ class EVENTS_EXPORT X11EventSource { ...@@ -79,6 +80,10 @@ class EVENTS_EXPORT X11EventSource {
void StopCurrentEventStream(); void StopCurrentEventStream();
void OnDispatcherListChanged(); void OnDispatcherListChanged();
// Explicitly asks the X11 server for the current timestamp, and updates
// |last_seen_server_time_| with this value.
Time GetCurrentServerTime();
protected: protected:
// Extracts cookie data from |xevent| if it's of GenericType, and dispatches // Extracts cookie data from |xevent| if it's of GenericType, and dispatches
// the event. This function also frees up the cookie data after dispatch is // the event. This function also frees up the cookie data after dispatch is
...@@ -88,10 +93,6 @@ class EVENTS_EXPORT X11EventSource { ...@@ -88,10 +93,6 @@ class EVENTS_EXPORT X11EventSource {
// Handles updates after event has been dispatched. // Handles updates after event has been dispatched.
void PostDispatchEvent(XEvent* xevent); void PostDispatchEvent(XEvent* xevent);
// Explicitly asks the X11 server for the current timestamp, and updates
// |last_seen_server_time_| with this value.
Time GetCurrentServerTime();
private: private:
static X11EventSource* instance_; static X11EventSource* instance_;
...@@ -115,6 +116,10 @@ class EVENTS_EXPORT X11EventSource { ...@@ -115,6 +116,10 @@ class EVENTS_EXPORT X11EventSource {
std::unique_ptr<X11HotplugEventHandler> hotplug_event_handler_; std::unique_ptr<X11HotplugEventHandler> hotplug_event_handler_;
// Used to sample RTT measurements, with frequency 1/1000.
std::default_random_engine generator_;
std::uniform_int_distribution<int> distribution_;
DISALLOW_COPY_AND_ASSIGN(X11EventSource); DISALLOW_COPY_AND_ASSIGN(X11EventSource);
}; };
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment