bug 1218576 - batch child Telemetry accumulations to limit IPC calls r?gfritzsche
Sending each accumulation one by one introduces some nasty talos regresisons.
So, let's batch them, and only send on periodic intervals.
Two notes:
1) Only start the timer when MIN_NUM_ACCUMULATIONS accumulations have
accumulated
2) Do not unarm the timer fully until after SendTelemetry completes
These both, independently, assist in ensuring that accumulations that happen
during SendTelemetry do not force us into a quick loop. That could result in
negative consequences from a power-consumption POV.
MozReview-Commit-ID: FWCejqhdJqu
--- a/dom/ipc/ContentParent.cpp
+++ b/dom/ipc/ContentParent.cpp
@@ -5890,21 +5890,23 @@ ContentParent::RecvNotifyPushSubscriptio
nsresult rv = pushNotifier->NotifySubscriptionModifiedObservers(aScope,
aPrincipal);
Unused << NS_WARN_IF(NS_FAILED(rv));
#endif
return true;
}
bool
-ContentParent::RecvTelemetry(const ID& aId,
- const nsCString& aKey,
- const uint32_t& aSample)
-{
- Telemetry::AccumulateChild(aId, aKey, aSample);
+ContentParent::RecvTelemetry(const nsTArray<Accumulation>& aAccumulations)
+{
+ for (uint32_t i = 0; i < aAccumulations.Length(); ++i) {
+ Telemetry::AccumulateChild(aAccumulations[i].mId,
+ aAccumulations[i].mKey,
+ aAccumulations[i].mSample);
+ }
return true;
}
} // namespace dom
} // namespace mozilla
NS_IMPL_ISUPPORTS(ParentIdleListener, nsIObserver)
--- a/dom/ipc/ContentParent.h
+++ b/dom/ipc/ContentParent.h
@@ -1111,19 +1111,17 @@ private:
InfallibleTArray<uint8_t>&& aData) override;
virtual bool RecvNotifyPushSubscriptionChangeObservers(const nsCString& aScope,
const IPC::Principal& aPrincipal) override;
virtual bool RecvNotifyPushSubscriptionModifiedObservers(const nsCString& aScope,
const IPC::Principal& aPrincipal) override;
- virtual bool RecvTelemetry(const ID& aId,
- const nsCString& aKey,
- const uint32_t& aSample) override;
+ virtual bool RecvTelemetry(const nsTArray<Accumulation>& aAccumulations) override;
// If you add strong pointers to cycle collected objects here, be sure to
// release these objects in ShutDownProcess. See the comment there for more
// details.
GeckoChildProcessHost* mSubprocess;
ContentParent* mOpener;
--- a/dom/ipc/PContent.ipdl
+++ b/dom/ipc/PContent.ipdl
@@ -93,17 +93,18 @@ using mozilla::gfx::IntSize from "mozill
using mozilla::dom::TabId from "mozilla/dom/ipc/IdType.h";
using mozilla::dom::ContentParentId from "mozilla/dom/ipc/IdType.h";
using mozilla::LayoutDeviceIntPoint from "Units.h";
using struct LookAndFeelInt from "mozilla/widget/WidgetMessageUtils.h";
using class mozilla::dom::ipc::StructuredCloneData from "ipc/IPCMessageUtils.h";
using mozilla::DataStorageType from "ipc/DataStorageIPCUtils.h";
using mozilla::DocShellOriginAttributes from "mozilla/ipc/BackgroundUtils.h";
using struct mozilla::layers::TextureFactoryIdentifier from "mozilla/layers/CompositorTypes.h";
-using mozilla::Telemetry::ID from "mozilla/TelemetryComms.h";
+using mozilla::Telemetry::TelemetryComms::Accumulation from "mozilla/TelemetryComms.h";
+using nsTArray<Accumulation> from "mozilla/TelemetryComms.h";
union ChromeRegistryItem
{
ChromePackage;
OverrideMapping;
SubstitutionMapping;
};
@@ -1201,22 +1202,21 @@ parent:
/**
* Notify `push-subscription-modified` observers in the parent.
*/
async NotifyPushSubscriptionModifiedObservers(nsCString scope,
Principal principal);
/**
- * Tells the parent to accumulate `sample` on the child histogram for `aId`
- * optionally keyed by `key` if it isn't voided.
+ * Tells the parent to accumulate `aAccumulations` as specified
*
* This is how content processes communicate their Telemetry
*/
- async Telemetry(ID aId, nsCString aKey, uint32_t aSample);
+ async Telemetry(nsTArray<Accumulation> aAccumulations);
both:
async AsyncMessage(nsString aMessage, CpowEntry[] aCpows,
Principal aPrincipal, ClonedMessageData aData);
};
}
}
--- a/toolkit/components/telemetry/TelemetryComms.cpp
+++ b/toolkit/components/telemetry/TelemetryComms.cpp
@@ -1,42 +1,71 @@
/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
/* vim: set ts=8 sts=2 et sw=2 tw=80: */
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
#include "TelemetryComms.h"
+#include "nsITimer.h"
#include "nsString.h"
#include "mozilla/unused.h"
#include "mozilla/dom/ContentChild.h"
namespace mozilla {
namespace Telemetry {
namespace TelemetryComms {
+#define MIN_NUM_ACCUMULATIONS 5
+#define BATCH_TIMEOUT_MS 2000
+
+static nsITimer* sCommTimer = nullptr;
+static nsTArray<Accumulation> accumulations;
+
bool RemoteAccumulate(mozilla::Telemetry::ID aId, uint32_t aSample)
{
static nsCString empty = EmptyCString();
if (!empty.IsVoid()) {
empty.SetIsVoid(true);
}
return RemoteAccumulate(aId, empty, aSample);
}
+static void submitBatchedAccumulations(nsITimer* aTimer, void* aClosure)
+{
+ // double-buffer in case SendTelemetry causes an accumulation
+ nsTArray<Accumulation> accumulationsToSend(accumulations);
+ accumulations.Clear();
+
+ mozilla::dom::ContentChild* parentActor = mozilla::dom::ContentChild::GetSingleton();
+ if (!NS_WARN_IF(!parentActor)) {
+ Unused << NS_WARN_IF(parentActor->SendTelemetry(accumulationsToSend));
+ }
+
+ // don't clear the timer until after SendTelemetry completes
+ // This should prevent us from waking up the thread when otherwise idle
+ // due to accumulations caused by SendTelemetry
+ NS_RELEASE(sCommTimer);
+}
+
bool RemoteAccumulate(mozilla::Telemetry::ID aId, const nsCString& aKey,
uint32_t aSample)
{
if (XRE_IsContentProcess()) {
- mozilla::dom::ContentChild* parentActor = mozilla::dom::ContentChild::GetSingleton();
- if (!NS_WARN_IF(!parentActor)) {
- Unused << NS_WARN_IF(parentActor->SendTelemetry(aId, aKey, aSample));
+ accumulations.AppendElement(Accumulation{aId, aKey, aSample});
+ if (!sCommTimer && accumulations.Length() > MIN_NUM_ACCUMULATIONS) {
+ CallCreateInstance(NS_TIMER_CONTRACTID, &sCommTimer);
+ if (sCommTimer)
+ sCommTimer->InitWithFuncCallback(submitBatchedAccumulations,
+ nullptr,
+ BATCH_TIMEOUT_MS,
+ nsITimer::TYPE_ONE_SHOT);
}
return true;
}
return false;
}
} // namespace TelemetryComms
} // namespace Telemetry
--- a/toolkit/components/telemetry/TelemetryComms.h
+++ b/toolkit/components/telemetry/TelemetryComms.h
@@ -10,36 +10,50 @@
namespace mozilla {
namespace Telemetry {
enum ID : uint32_t;
namespace TelemetryComms {
+struct Accumulation
+{
+ mozilla::Telemetry::ID mId;
+ nsCString mKey;
+ uint32_t mSample;
+};
+
bool RemoteAccumulate(mozilla::Telemetry::ID aId, uint32_t aSample);
bool RemoteAccumulate(mozilla::Telemetry::ID aId, const nsCString& aKey, uint32_t aSample);
} // namespace TelemetryComms
} // namespace Telemetry
} // namespace mozilla
namespace IPC {
template <>
-struct ParamTraits<mozilla::Telemetry::ID>
+struct ParamTraits<mozilla::Telemetry::TelemetryComms::Accumulation>
{
- typedef mozilla::Telemetry::ID paramType;
+ typedef mozilla::Telemetry::TelemetryComms::Accumulation paramType;
static void Write(Message* aMsg, const paramType& aParam)
{
- aMsg->WriteUInt32(aParam);
+ aMsg->WriteUInt32(aParam.mId);
+ WriteParam(aMsg, aParam.mKey);
+ WriteParam(aMsg, aParam.mSample);
}
static bool Read(const Message* aMsg, void** aIter, paramType* aResult)
{
- return aMsg->ReadUInt32(aIter, reinterpret_cast<uint32_t*>(aResult));
+ if (!aMsg->ReadUInt32(aIter, reinterpret_cast<uint32_t*>(&(aResult->mId))) ||
+ !ReadParam(aMsg, aIter, &(aResult->mKey)) ||
+ !ReadParam(aMsg, aIter, &(aResult->mSample)))
+ return false;
+
+ return true;
}
};
} // namespace IPC
#endif // Telemetry_Comms_h__