From 217e25cfa5ecb1c9ed0ba2c454327ddcc68949fc Mon Sep 17 00:00:00 2001 From: Chris H-C Date: Wed, 1 Dec 2021 14:35:08 +0000 Subject: [PATCH] Bug 1729026 - Add support for GPU-process Glean metrics via FOG r=Dexter,nical Differential Revision: https://phabricator.services.mozilla.com/D124840 --- gfx/ipc/GPUChild.cpp | 6 ++++ gfx/ipc/GPUChild.h | 1 + gfx/ipc/GPUParent.cpp | 11 +++++++ gfx/ipc/GPUParent.h | 2 ++ gfx/ipc/PGPU.ipdl | 11 +++++++ toolkit/components/glean/api/src/ipc.rs | 3 ++ toolkit/components/glean/docs/dev/ipc.md | 1 + toolkit/components/glean/ipc/FOGIPC.cpp | 37 ++++++++++++++++++++---- toolkit/components/glean/metrics.yaml | 18 ++++++++++++ 9 files changed, 84 insertions(+), 6 deletions(-) diff --git a/gfx/ipc/GPUChild.cpp b/gfx/ipc/GPUChild.cpp index 3f126f9b48e5..d0c4109e1ffc 100644 --- a/gfx/ipc/GPUChild.cpp +++ b/gfx/ipc/GPUChild.cpp @@ -12,6 +12,7 @@ #include "gfxConfig.h" #include "gfxPlatform.h" #include "mozilla/Components.h" +#include "mozilla/FOGIPC.h" #include "mozilla/StaticPrefs_dom.h" #include "mozilla/Telemetry.h" #include "mozilla/TelemetryIPC.h" @@ -316,6 +317,11 @@ mozilla::ipc::IPCResult GPUChild::RecvUpdateMediaCodecsSupported( return IPC_OK(); } +mozilla::ipc::IPCResult GPUChild::RecvFOGData(ByteBuf&& aBuf) { + glean::FOGData(std::move(aBuf)); + return IPC_OK(); +} + class DeferredDeleteGPUChild : public Runnable { public: explicit DeferredDeleteGPUChild(UniquePtr&& aChild) diff --git a/gfx/ipc/GPUChild.h b/gfx/ipc/GPUChild.h index 4ff9e13dd5ed..021bd216700e 100644 --- a/gfx/ipc/GPUChild.h +++ b/gfx/ipc/GPUChild.h @@ -72,6 +72,7 @@ class GPUChild final : public ipc::CrashReporterHelper, mozilla::ipc::IPCResult RecvBHRThreadHang(const HangDetails& aDetails); mozilla::ipc::IPCResult RecvUpdateMediaCodecsSupported( const PDMFactory::MediaCodecsSupported& aSupported); + mozilla::ipc::IPCResult RecvFOGData(ByteBuf&& aBuf); bool SendRequestMemoryReport(const uint32_t& aGeneration, const bool& aAnonymize, diff --git a/gfx/ipc/GPUParent.cpp b/gfx/ipc/GPUParent.cpp index 084e5feae1c9..7d5358a78874 100644 --- a/gfx/ipc/GPUParent.cpp +++ b/gfx/ipc/GPUParent.cpp @@ -25,6 +25,7 @@ #include "gfxPlatform.h" #include "mozilla/Assertions.h" #include "mozilla/Components.h" +#include "mozilla/FOGIPC.h" #include "mozilla/HangDetails.h" #include "mozilla/PerfStats.h" #include "mozilla/Preferences.h" @@ -617,12 +618,22 @@ mozilla::ipc::IPCResult GPUParent::RecvCollectPerfStatsJSON( return IPC_OK(); } +mozilla::ipc::IPCResult GPUParent::RecvFlushFOGData( + FlushFOGDataResolver&& aResolver) { + glean::FlushFOGData(std::move(aResolver)); + return IPC_OK(); +} + void GPUParent::ActorDestroy(ActorDestroyReason aWhy) { if (AbnormalShutdown == aWhy) { NS_WARNING("Shutting down GPU process early due to a crash!"); ProcessChild::QuickExit(); } + // Send the last bits of Glean data over to the main process. + glean::FlushFOGData( + [](ByteBuf&& aBuf) { glean::SendFOGData(std::move(aBuf)); }); + #ifndef NS_FREE_PERMANENT_DATA # ifdef XP_WIN wmf::MFShutdown(); diff --git a/gfx/ipc/GPUParent.h b/gfx/ipc/GPUParent.h index 8fa4a3c7c589..8456690599f7 100644 --- a/gfx/ipc/GPUParent.h +++ b/gfx/ipc/GPUParent.h @@ -97,6 +97,8 @@ class GPUParent final : public PGPUParent { Endpoint&& aEndpoint); #endif + mozilla::ipc::IPCResult RecvFlushFOGData(FlushFOGDataResolver&& aResolver); + void ActorDestroy(ActorDestroyReason aWhy) override; private: diff --git a/gfx/ipc/PGPU.ipdl b/gfx/ipc/PGPU.ipdl index 789f15b1cb77..85a942cca60b 100644 --- a/gfx/ipc/PGPU.ipdl +++ b/gfx/ipc/PGPU.ipdl @@ -22,6 +22,7 @@ include protocol PRemoteDecoderManager; include protocol PSandboxTesting; #endif +include "mozilla/ipc/ByteBufUtils.h"; include "mozilla/layers/LayersMessageUtils.h"; using base::ProcessId from "base/process.h"; @@ -112,6 +113,11 @@ parent: async InitSandboxTesting(Endpoint aEndpoint); #endif + // Tells the gpu process to flush any pending telemetry. + // Used in tests and ping assembly. Buffer contains bincoded Rust structs. + // https://firefox-source-docs.mozilla.org/toolkit/components/glean/dev/ipc.html + async FlushFOGData() returns (ByteBuf buf); + child: // Sent when the GPU process has initialized devices. This occurs once, after // Init(). @@ -158,6 +164,11 @@ child: // Update the cached list of codec supported following a check in the // GPU parent. async UpdateMediaCodecsSupported(MediaCodecsSupported aSupported); + + // Sent from time-to-time to limit the amount of telemetry vulnerable to loss + // Buffer contains bincoded Rust structs. + // https://firefox-source-docs.mozilla.org/toolkit/components/glean/dev/ipc.html + async FOGData(ByteBuf buf); }; } // namespace gfx diff --git a/toolkit/components/glean/api/src/ipc.rs b/toolkit/components/glean/api/src/ipc.rs index 973b1d9a2f2b..d432b8010008 100644 --- a/toolkit/components/glean/api/src/ipc.rs +++ b/toolkit/components/glean/api/src/ipc.rs @@ -97,6 +97,9 @@ fn register_process_shutdown(process_type: u32) { FOG_RegisterContentChildShutdown(); }; } + nsIXULRuntime::PROCESS_TYPE_GPU => { + // GPU process shutdown is handled in GPUParent::ActorDestroy. + } _ => { // We don't yet support other process types. log::error!("Process type {} tried to use FOG, but isn't supported! (Process type constants are in nsIXULRuntime.rs)", process_type); diff --git a/toolkit/components/glean/docs/dev/ipc.md b/toolkit/components/glean/docs/dev/ipc.md index 4b9aadb1060c..fe8e7f56fd1b 100644 --- a/toolkit/components/glean/docs/dev/ipc.md +++ b/toolkit/components/glean/docs/dev/ipc.md @@ -127,6 +127,7 @@ Rust is then responsible for turning the pending data into FOG supports messaging between the following types of child process and the parent process: * content children (via `PContent` (for now. See [bug 1641989](https://bugzilla.mozilla.org/show_bug.cgi?id=1641989)) +* gpu children (via `PGPU`) See [the process model docs](../../../../dom/ipc/process_model.html) diff --git a/toolkit/components/glean/ipc/FOGIPC.cpp b/toolkit/components/glean/ipc/FOGIPC.cpp index 90898a910978..731ded396921 100644 --- a/toolkit/components/glean/ipc/FOGIPC.cpp +++ b/toolkit/components/glean/ipc/FOGIPC.cpp @@ -9,12 +9,18 @@ #include "mozilla/glean/GleanMetrics.h" #include "mozilla/dom/ContentChild.h" #include "mozilla/dom/ContentParent.h" +#include "mozilla/gfx/GPUChild.h" +#include "mozilla/gfx/GPUParent.h" +#include "mozilla/gfx/GPUProcessManager.h" #include "mozilla/MozPromise.h" #include "mozilla/ProcInfo.h" +#include "mozilla/Unused.h" #include "nsTArray.h" #include "nsThreadUtils.h" using mozilla::dom::ContentParent; +using mozilla::gfx::GPUChild; +using mozilla::gfx::GPUProcessManager; using mozilla::ipc::ByteBuf; using FlushFOGDataPromise = mozilla::dom::ContentParent::FlushFOGDataPromise; @@ -81,19 +87,33 @@ void FlushFOGData(std::function&& aResolver) { */ void FlushAllChildData( std::function&&)>&& aResolver) { + auto timerId = fog_ipc::flush_durations.Start(); + nsTArray parents; ContentParent::GetAll(parents); - if (parents.Length() == 0) { + nsTArray> promises; + for (auto* parent : parents) { + promises.EmplaceBack(parent->SendFlushFOGData()); + } + + GPUProcessManager* gpuManager = GPUProcessManager::Get(); + GPUChild* gpuChild = nullptr; + if (gpuManager) { + gpuChild = gpuManager->GetGPUChild(); + if (gpuChild) { + promises.EmplaceBack(gpuChild->SendFlushFOGData()); + } + } + + if (promises.Length() == 0) { + // No child processes at the moment. Resolve synchronously. + fog_ipc::flush_durations.Cancel(std::move(timerId)); nsTArray results; aResolver(std::move(results)); return; } - auto timerId = fog_ipc::flush_durations.Start(); - nsTArray> promises; - for (auto* parent : parents) { - promises.EmplaceBack(parent->SendFlushFOGData()); - } + // If fog.ipc.flush_failures ever gets too high: // TODO: Don't throw away resolved data if some of the promises reject. // (not sure how, but it'll mean not using ::All... maybe a custom copy of // AllPromiseHolder? Might be impossible outside MozPromise.h) @@ -106,6 +126,7 @@ void FlushAllChildData( if (aValue.IsResolve()) { aResolver(std::move(aValue.ResolveValue())); } else { + fog_ipc::flush_failures.Add(1); nsTArray results; aResolver(std::move(results)); } @@ -130,6 +151,10 @@ void SendFOGData(ipc::ByteBuf&& buf) { case GeckoProcessType_Content: mozilla::dom::ContentChild::GetSingleton()->SendFOGData(std::move(buf)); break; + case GeckoProcessType_GPU: + Unused << mozilla::gfx::GPUParent::GetSingleton()->SendFOGData( + std::move(buf)); + break; default: MOZ_ASSERT_UNREACHABLE("Unsuppored process type"); } diff --git a/toolkit/components/glean/metrics.yaml b/toolkit/components/glean/metrics.yaml index 305a63976b89..12eaed6fb6be 100644 --- a/toolkit/components/glean/metrics.yaml +++ b/toolkit/components/glean/metrics.yaml @@ -97,3 +97,21 @@ fog.ipc: - chutten@mozilla.com - glean-team@mozilla.com expires: never + + flush_failures: + type: counter + description: | + The number of times we failed to flush all non-parent-process data, + throwing even partial results into the trash. + If this number is high, we might consider writing custom `MozPromise`- + handling code instead of using `MozPromise::All`. + bugs: + - https://bugzilla.mozilla.org/show_bug.cgi?id=1729026 + data_reviews: + - https://bugzilla.mozilla.org/show_bug.cgi?id=1729026 + data_sensitivity: + - technical + notification_emails: + - chutten@mozilla.com + - glean-team@mozilla.com + expires: never