New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 685205 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Jan 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Bug

Blocking:
issue 620852



Sign in to add a comment

Make URLFetcherCore::ReleaseRequest() release IOBuffer |buffer_|

Project Member Reported by xunji...@chromium.org, Jan 25 2017

Issue description

URLFetcherCore::ReleaseRequest() releases net::URLRequest when request is done or canceled. URLFetcherCore can release its 4KiB |buffer_| too.

This comes up in manual instrumentation. net/ MemoryDumpProvider shows that there are no active requests, but URLFetcherCore::|buffer_| consumes 4KiB each even though it is no longer needed.

void URLFetcherCore::ReleaseRequest() {
  request_context_getter_->RemoveObserver(this);
  upload_progress_checker_timer_.reset();
  request_.reset();
+ buffer_ = nullptr;
  g_registry.Get().RemoveURLFetcherCore(this);
}
 
url_fetcher.png
44.5 KB View Download

Comment 1 by mmenke@chromium.org, Jan 25 2017

Can we just have the consumers delete the URLFetchers?  It keeps a lot of other metadata around as well (Headers, for instance, and be quite large).
Blocking: 620852
Labels: Performance-Memory
SGTM. I can file a bug against chrome/browser/safe_browsing/client_side_model_loader.cc. I think we can fix |buffer_| on our side, since that's the majority of the memory taken by URLFetcher.
I filed  Issue 685235  on safe browsing.

Project Member

Comment 4 by bugdroid1@chromium.org, Jan 25 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3c5b7638f0662fb0a8fc1e3ef1ec556232dee0b6

commit 3c5b7638f0662fb0a8fc1e3ef1ec556232dee0b6
Author: xunjieli <xunjieli@chromium.org>
Date: Wed Jan 25 19:13:34 2017

Make URLFetcherCore::ReleaseRequest() release |buffer_|.

Consumer might hold on to URLFetcher while net::URLRequest is
gone. URLFetcherCore::|buffer_| consume 4KiB each if not
deallocated. This CL makes ReleaseRequest() null out |buffer_|
too and moves the initialization of |buffer_| to
OnResponseStart() because URLFetcherCore can retry on 5xx and
on network change.

R=mmenke@chromium.org

BUG= 685205 

Review-Url: https://codereview.chromium.org/2653183006
Cr-Commit-Position: refs/heads/master@{#446078}

[modify] https://crrev.com/3c5b7638f0662fb0a8fc1e3ef1ec556232dee0b6/net/url_request/url_fetcher_core.cc

Status: Fixed (was: Started)

Sign in to add a comment