Bug 1662072 - Reduce one data copy in parent process when sending chunks to child process r=valentin

1. The data sent from `HttpBackgroundChannelParent` to `HttpBackgroundChannelChild` is using type `nsDependentCSubstring`, since I'd like to reduce one copy in parent process.
2. The data sent from `HttpTransactionChild` to `HttpTransactionParent` is using type `nsCString`. The main reason is that the data is actually captured in `HttpTransactionParent::RecvOnDataAvailable` [1] and put in channel event queue. If we use `nsDependentCSubstring` here, the data would be copied in parent process.
3.  In `HttpBackgroundChannelChild::RecvOnTransportAndData`, it's inevitable that the data is copied when assigning the data to a `nsCString`, since sometimes `HttpBackgroundChannelChild::RecvOnTransportAndData` needs to be queued and execute later.


[1] https://searchfox.org/mozilla-central/rev/969fc7fa6c3c7fc489f53b7b7f8c902028b5169f/netwerk/protocol/http/HttpTransactionParent.cpp#556

Differential Revision: https://phabricator.services.mozilla.com/D88781
This commit is contained in:
Kershaw Chang 2020-08-31 17:11:21 +00:00
parent a193e2372f
commit d7d4941cf6
10 changed files with 79 additions and 73 deletions

View File

@ -21,7 +21,8 @@ void BackgroundDataBridgeChild::ActorDestroy(ActorDestroyReason aWhy) {
}
mozilla::ipc::IPCResult BackgroundDataBridgeChild::RecvOnTransportAndData(
const uint64_t& offset, const uint32_t& count, const nsCString& data) {
const uint64_t& offset, const uint32_t& count,
const nsDependentCSubstring& data) {
if (!mBgChild) {
return IPC_OK();
}

View File

@ -25,9 +25,9 @@ class BackgroundDataBridgeChild final : public PBackgroundDataBridgeChild {
RefPtr<HttpBackgroundChannelChild> mBgChild;
public:
mozilla::ipc::IPCResult RecvOnTransportAndData(const uint64_t& offset,
const uint32_t& count,
const nsCString& data);
mozilla::ipc::IPCResult RecvOnTransportAndData(
const uint64_t& offset, const uint32_t& count,
const nsDependentCSubstring& data);
mozilla::ipc::IPCResult RecvOnStopRequest(
nsresult aStatus, const ResourceTimingStructArgs& aTiming,
const TimeStamp& aLastActiveTabOptHit,

View File

@ -213,31 +213,42 @@ IPCResult HttpBackgroundChannelChild::RecvOnStartRequest(
IPCResult HttpBackgroundChannelChild::RecvOnTransportAndData(
const nsresult& aChannelStatus, const nsresult& aTransportStatus,
const uint64_t& aOffset, const uint32_t& aCount, const nsCString& aData,
const bool& aDataFromSocketProcess) {
LOG(
("HttpBackgroundChannelChild::RecvOnTransportAndData [this=%p, "
"aDataFromSocketProcess=%d, mFirstODASource=%d]\n",
this, aDataFromSocketProcess, mFirstODASource));
MOZ_ASSERT(OnSocketThread());
const uint64_t& aOffset, const uint32_t& aCount,
const nsDependentCSubstring& aData, const bool& aDataFromSocketProcess) {
RefPtr<HttpBackgroundChannelChild> self = this;
nsCString data(aData);
std::function<void()> callProcessOnTransportAndData =
[self, aChannelStatus, aTransportStatus, aOffset, aCount, data,
aDataFromSocketProcess]() {
LOG(
("HttpBackgroundChannelChild::RecvOnTransportAndData [this=%p, "
"aDataFromSocketProcess=%d, mFirstODASource=%d]\n",
self.get(), aDataFromSocketProcess, self->mFirstODASource));
MOZ_ASSERT(OnSocketThread());
if (NS_WARN_IF(!mChannelChild)) {
return IPC_OK();
}
if (NS_WARN_IF(!self->mChannelChild)) {
return;
}
if (((mFirstODASource == ODA_FROM_SOCKET) && !aDataFromSocketProcess) ||
((mFirstODASource == ODA_FROM_PARENT) && aDataFromSocketProcess)) {
return IPC_OK();
}
if (((self->mFirstODASource == ODA_FROM_SOCKET) &&
!aDataFromSocketProcess) ||
((self->mFirstODASource == ODA_FROM_PARENT) &&
aDataFromSocketProcess)) {
return;
}
// The HttpTransactionChild in socket process may not know that this request
// is cancelled or failed due to the IPC delay. In this case, we should not
// forward ODA to HttpChannelChild.
nsresult channelStatus;
mChannelChild->GetStatus(&channelStatus);
if (NS_FAILED(channelStatus)) {
return IPC_OK();
}
// The HttpTransactionChild in socket process may not know that this
// request is cancelled or failed due to the IPC delay. In this case, we
// should not forward ODA to HttpChannelChild.
nsresult channelStatus;
self->mChannelChild->GetStatus(&channelStatus);
if (NS_FAILED(channelStatus)) {
return;
}
self->mChannelChild->ProcessOnTransportAndData(
aChannelStatus, aTransportStatus, aOffset, aCount, data);
};
// Bug 1641336: Race only happens if the data is from socket process.
if (IsWaitingOnStartRequest()) {
@ -245,19 +256,13 @@ IPCResult HttpBackgroundChannelChild::RecvOnTransportAndData(
"]\n",
aOffset, aCount));
mQueuedRunnables.AppendElement(
NewRunnableMethod<const nsresult, const nsresult, const uint64_t,
const uint32_t, const nsCString, bool>(
"HttpBackgroundChannelChild::RecvOnTransportAndData", this,
&HttpBackgroundChannelChild::RecvOnTransportAndData, aChannelStatus,
aTransportStatus, aOffset, aCount, aData, aDataFromSocketProcess));
mQueuedRunnables.AppendElement(NS_NewRunnableFunction(
"HttpBackgroundChannelChild::RecvOnTransportAndData",
std::move(callProcessOnTransportAndData)));
return IPC_OK();
}
mChannelChild->ProcessOnTransportAndData(aChannelStatus, aTransportStatus,
aOffset, aCount, aData);
callProcessOnTransportAndData();
return IPC_OK();
}
@ -484,8 +489,8 @@ IPCResult HttpBackgroundChannelChild::RecvSetClassifierMatchedTrackingInfo(
return IPC_OK();
}
// SetClassifierMatchedTrackingInfo has no order dependency to OnStartRequest.
// It this be handled as soon as possible
// SetClassifierMatchedTrackingInfo has no order dependency to
// OnStartRequest. It this be handled as soon as possible
mChannelChild->ProcessSetClassifierMatchedTrackingInfo(info.list(),
info.fullhash());

View File

@ -60,7 +60,7 @@ class HttpBackgroundChannelChild final : public PHttpBackgroundChannelChild {
const nsresult& aTransportStatus,
const uint64_t& aOffset,
const uint32_t& aCount,
const nsCString& aData,
const nsDependentCSubstring& aData,
const bool& aDataFromSocketProcess);
IPCResult RecvOnStopRequest(

View File

@ -198,9 +198,9 @@ bool HttpBackgroundChannelParent::OnTransportAndData(
return NS_SUCCEEDED(rv);
}
std::function<bool(const nsCString&, uint64_t, uint32_t)> sendFunc =
nsHttp::SendFunc<nsDependentCSubstring> sendFunc =
[self = UnsafePtr<HttpBackgroundChannelParent>(this), aChannelStatus,
aTransportStatus](const nsCString& aData, uint64_t aOffset,
aTransportStatus](const nsDependentCSubstring& aData, uint64_t aOffset,
uint32_t aCount) {
return self->SendOnTransportAndData(aChannelStatus, aTransportStatus,
aOffset, aCount, aData, false);

View File

@ -290,7 +290,7 @@ HttpTransactionChild::OnDataAvailable(nsIRequest* aRequest,
return NS_ERROR_FAILURE;
}
std::function<bool(const nsCString&, uint64_t, uint32_t)> sendFunc =
nsHttp::SendFunc<nsCString> sendFunc =
[self = UnsafePtr<HttpTransactionChild>(this)](
const nsCString& aData, uint64_t aOffset, uint32_t aCount) {
return self->SendOnDataAvailable(aData, aOffset, aCount);
@ -310,9 +310,10 @@ HttpTransactionChild::OnDataAvailable(nsIRequest* aRequest,
return NS_ERROR_FAILURE;
}
std::function<bool(const nsCString&, uint64_t, uint32_t)> sendFunc =
nsHttp::SendFunc<nsDependentCSubstring> sendFunc =
[self = UnsafePtr<HttpTransactionChild>(this)](
const nsCString& aData, uint64_t aOffset, uint32_t aCount) {
const nsDependentCSubstring& aData, uint64_t aOffset,
uint32_t aCount) {
return self->mDataBridgeParent->SendOnTransportAndData(aOffset, aCount,
aData);
};
@ -331,7 +332,7 @@ HttpTransactionChild::OnDataAvailable(nsIRequest* aRequest,
NS_NewRunnableFunction(
"HttpTransactionChild::OnDataAvailable",
[self, offset(aOffset), count(aCount), data(data)]() {
std::function<bool(const nsCString&, uint64_t, uint32_t)> sendFunc =
nsHttp::SendFunc<nsCString> sendFunc =
[self](const nsCString& aData, uint64_t aOffset,
uint32_t aCount) {
return self->SendOnDataAvailable(aData, aOffset, aCount);

View File

@ -17,7 +17,7 @@ async refcounted protocol PBackgroundDataBridge
child:
async OnTransportAndData(uint64_t offset,
uint32_t count,
nsCString data);
nsDependentCSubstring data);
async OnStopRequest(nsresult aStatus,
ResourceTimingStructArgs timing,

View File

@ -35,7 +35,7 @@ child:
nsresult transportStatus,
uint64_t offset,
uint32_t count,
nsCString data,
nsDependentCSubstring data,
bool dataFromSocketProcess);

View File

@ -560,29 +560,6 @@ nsCString ConvertRequestHeadToString(nsHttpRequestHead& aRequestHead,
return reqHeaderBuf;
}
bool SendDataInChunks(const nsCString& aData, uint64_t aOffset, uint32_t aCount,
const std::function<bool(const nsCString&, uint64_t,
uint32_t)>& aSendFunc) {
static uint32_t const kCopyChunkSize = 128 * 1024;
uint32_t toRead = std::min<uint32_t>(aCount, kCopyChunkSize);
uint32_t start = 0;
while (aCount) {
nsCString data(Substring(aData, start, toRead));
if (!aSendFunc(data, aOffset, toRead)) {
return false;
}
aOffset += toRead;
start += toRead;
aCount -= toRead;
toRead = std::min<uint32_t>(aCount, kCopyChunkSize);
}
return true;
}
void NotifyActiveTabLoadOptimization() {
if (gHttpHandler) {
gHttpHandler->NotifyActiveTabLoadOptimization();

View File

@ -264,9 +264,31 @@ nsCString ConvertRequestHeadToString(nsHttpRequestHead& aRequestHead,
bool aRequestBodyHasHeaders,
bool aUsingConnect);
bool SendDataInChunks(
const nsCString& aData, uint64_t aOffset, uint32_t aCount,
const std::function<bool(const nsCString&, uint64_t, uint32_t)>& aSendFunc);
template <typename T>
using SendFunc = std::function<bool(const T&, uint64_t, uint32_t)>;
template <typename T>
bool SendDataInChunks(const nsCString& aData, uint64_t aOffset, uint32_t aCount,
const SendFunc<T>& aSendFunc) {
static uint32_t const kCopyChunkSize = 128 * 1024;
uint32_t toRead = std::min<uint32_t>(aCount, kCopyChunkSize);
uint32_t start = 0;
while (aCount) {
T data(Substring(aData, start, toRead));
if (!aSendFunc(data, aOffset, toRead)) {
return false;
}
aOffset += toRead;
start += toRead;
aCount -= toRead;
toRead = std::min<uint32_t>(aCount, kCopyChunkSize);
}
return true;
}
} // namespace nsHttp