Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2650943007:
Track QuicChromiumClientSession in net/ MemoryDumpProvider (Closed)
DescriptionTrack QuicChromiumClientSession in net/ MemoryDumpProvider
This CL tracks the number of QuicChromiumClientSession in net/
MemoryDumpProvider. Follow-up CLs will improve the coverage of the memory
usage estimate.
BUG=669108
Review-Url: https://codereview.chromium.org/2650943007
Cr-Commit-Position: refs/heads/master@{#446986}
Committed: https://chromium.googlesource.com/chromium/src/+/69720dd78be79fe965b684ef4a753e3947d20317
Patch Set 1 : self review #Patch Set 2 : self review #
Total comments: 12
Patch Set 3 : address ssid comments #
Total comments: 4
Patch Set 4 : Address Dmitry and Ryan comments #
Total comments: 6
Patch Set 5 : Remove sizeof(port_) and FIXME #Patch Set 6 : self review #
Total comments: 2
Patch Set 7 : remove dep on upstream file and address zhongyi comment #Patch Set 8 : self review #
Total comments: 1
Patch Set 9 : be more specific in comment #
Total comments: 2
Messages
Total messages: 36 (14 generated)
Patchset #1 (id:1) has been deleted
xunjieli@chromium.org changed reviewers: + ssid@chromium.org, zhongyi@chromium.org
Cherie and Sid, PTAL. Thanks
Description was changed from ========== Track QuicChromiumClientSession in net/ MemoryDumpProvider This CL tracks number of QuicChromiumClientSession in net/ MemoryDumpProvider. Follow-up CLs will improve the coverage of the memory usage estimate. BUG=669108 ========== to ========== Track QuicChromiumClientSession in net/ MemoryDumpProvider This CL tracks the number of QuicChromiumClientSession in net/ MemoryDumpProvider. Follow-up CLs will improve the coverage of the memory usage estimate. BUG=669108 ==========
I'm on triage today, I will get back to this tomorrow.
Sid, a friendly ping!
xunjieli@chromium.org changed reviewers: + dskiba@chromium.org
dskiba@: One question about EstimateMemoryUsage for std::map below. Thanks! https://codereview.chromium.org/2650943007/diff/40001/net/quic/chromium/quic_... File net/quic/chromium/quic_stream_factory.cc (right): https://codereview.chromium.org/2650943007/diff/40001/net/quic/chromium/quic_... net/quic/chromium/quic_stream_factory.cc:1074: // account for |destination_| and |server_id_| here? Dmitry: so |all_sessions_| is a std::map of QuicChromiumClientStream to QuicSessionKey. Does EMU for std::map<QuicChromiumClientStream*, QuicSessionKey> already taken into account the size of QuicSessionKey?
looks good % the fixes. https://codereview.chromium.org/2650943007/diff/40001/net/quic/chromium/quic_... File net/quic/chromium/quic_chromium_packet_reader.cc (right): https://codereview.chromium.org/2650943007/diff/40001/net/quic/chromium/quic_... net/quic/chromium/quic_chromium_packet_reader.cc:70: // For now, just return the size of |read_buffer_|. If there is going to be a fix in future maybe add a TODO? https://codereview.chromium.org/2650943007/diff/40001/net/quic/chromium/quic_... File net/quic/chromium/quic_stream_factory.cc (right): https://codereview.chromium.org/2650943007/diff/40001/net/quic/chromium/quic_... net/quic/chromium/quic_stream_factory.cc:910: size_t estimate_memory = nit: memory_estimate or memory_usage or estimated_memory? https://codereview.chromium.org/2650943007/diff/40001/net/quic/chromium/quic_... net/quic/chromium/quic_stream_factory.cc:1072: // FIXME: Other comments in this file are TODO. Add TODO to be consistent? Or remove after fixing. https://codereview.chromium.org/2650943007/diff/40001/net/quic/chromium/quic_... net/quic/chromium/quic_stream_factory.cc:1074: // account for |destination_| and |server_id_| here? On 2017/01/25 21:50:38, xunjieli wrote: > Dmitry: so |all_sessions_| is a std::map of QuicChromiumClientStream to > QuicSessionKey. > Does EMU for std::map<QuicChromiumClientStream*, QuicSessionKey> already taken > into account the size of QuicSessionKey? Yes EstimateMemoryUsage will account for the sizeof(QuicSessionKey). But it does not account for the memory allocated by member: HostPortPair::host_ which is std::sting. It only accounted for sizeof(std::string) but not the actual string itself. But since the string just contains an ipv6 address (guess 16 bytes?), we can just ignore that allocation since it's small. Or if it can hold something bigger, you can return |EstimateMemoryUsage(destination_.host()) + EstimateMemoryUsage(server_id_.host_port_pair().host())| here.
Thanks, PTAL. https://codereview.chromium.org/2650943007/diff/40001/net/quic/chromium/quic_... File net/quic/chromium/quic_chromium_packet_reader.cc (right): https://codereview.chromium.org/2650943007/diff/40001/net/quic/chromium/quic_... net/quic/chromium/quic_chromium_packet_reader.cc:70: // For now, just return the size of |read_buffer_|. On 2017/01/25 22:42:04, ssid wrote: > If there is going to be a fix in future maybe add a TODO? Done. I changed the wording. I don't think we care about things other than the buffer. The rest seem small and didn't show up in native heap profiling traces. https://codereview.chromium.org/2650943007/diff/40001/net/quic/chromium/quic_... File net/quic/chromium/quic_stream_factory.cc (right): https://codereview.chromium.org/2650943007/diff/40001/net/quic/chromium/quic_... net/quic/chromium/quic_stream_factory.cc:910: size_t estimate_memory = On 2017/01/25 22:42:04, ssid wrote: > nit: memory_estimate or memory_usage or estimated_memory? Done. https://codereview.chromium.org/2650943007/diff/40001/net/quic/chromium/quic_... net/quic/chromium/quic_stream_factory.cc:1072: // FIXME: On 2017/01/25 22:42:04, ssid wrote: > Other comments in this file are TODO. Add TODO to be consistent? Or remove after > fixing. Done. Removed https://codereview.chromium.org/2650943007/diff/40001/net/quic/chromium/quic_... net/quic/chromium/quic_stream_factory.cc:1074: // account for |destination_| and |server_id_| here? On 2017/01/25 22:42:04, ssid wrote: > On 2017/01/25 21:50:38, xunjieli wrote: > > Dmitry: so |all_sessions_| is a std::map of QuicChromiumClientStream to > > QuicSessionKey. > > Does EMU for std::map<QuicChromiumClientStream*, QuicSessionKey> already taken > > into account the size of QuicSessionKey? > > Yes EstimateMemoryUsage will account for the sizeof(QuicSessionKey). But it does > not account for the memory allocated by member: HostPortPair::host_ which is > std::sting. > It only accounted for sizeof(std::string) but not the actual string itself. But > since the string just contains an ipv6 address (guess 16 bytes?), we can just > ignore that allocation since it's small. > Or if it can hold something bigger, you can return > > |EstimateMemoryUsage(destination_.host()) + > EstimateMemoryUsage(server_id_.host_port_pair().host())| here. Done. got it, thanks!
lgtm, thanks! https://codereview.chromium.org/2650943007/diff/40001/net/quic/chromium/quic_... File net/quic/chromium/quic_stream_factory.cc (right): https://codereview.chromium.org/2650943007/diff/40001/net/quic/chromium/quic_... net/quic/chromium/quic_stream_factory.cc:1074: // account for |destination_| and |server_id_| here? Just checking. Are you sure that the strings are small. The comment says "represents an IPv6 address".. But it can be represented in any way. the smallest I can imagine is 16 bytes.
https://codereview.chromium.org/2650943007/diff/40001/net/quic/chromium/quic_... File net/quic/chromium/quic_stream_factory.cc (right): https://codereview.chromium.org/2650943007/diff/40001/net/quic/chromium/quic_... net/quic/chromium/quic_stream_factory.cc:1074: // account for |destination_| and |server_id_| here? On 2017/01/25 21:50:38, xunjieli wrote: > Dmitry: so |all_sessions_| is a std::map of QuicChromiumClientStream to > QuicSessionKey. > Does EMU for std::map<QuicChromiumClientStream*, QuicSessionKey> already taken > into account the size of QuicSessionKey? So, the convention is that EMU(T) returns "additional" memory beyond the sizeof(T). Classes that contain QuicSessionKey will account for its sizeof(). For example EMU(unique_ptr<T>) is sizeof(T) + EMU(T). The easiest is just to call EMU() on all members that can allocate, without thinking of how big or small they can get. So I would include both destination_ and server_id_ here.
rch@chromium.org changed reviewers: + rch@chromium.org
https://codereview.chromium.org/2650943007/diff/60001/net/quic/chromium/quic_... File net/quic/chromium/quic_chromium_client_session.cc (right): https://codereview.chromium.org/2650943007/diff/60001/net/quic/chromium/quic_... net/quic/chromium/quic_chromium_client_session.cc:1525: // QuicHeaderList and QuicSession's QuiCWriteBlockedList. Would you also want to estimate any open streams? Would you also want to estimate the memory usage from the unacked packet map? (packets which have been sent but not received, in which case we need to keep the data around in case we need to retransmit it)
Patchset #4 (id:80001) has been deleted
Thanks! Two questions for Ryan below and one for Dmitry. PTAL https://codereview.chromium.org/2650943007/diff/40001/net/quic/chromium/quic_... File net/quic/chromium/quic_stream_factory.cc (right): https://codereview.chromium.org/2650943007/diff/40001/net/quic/chromium/quic_... net/quic/chromium/quic_stream_factory.cc:1074: // account for |destination_| and |server_id_| here? On 2017/01/25 23:39:57, DmitrySkiba wrote: > On 2017/01/25 21:50:38, xunjieli wrote: > > Dmitry: so |all_sessions_| is a std::map of QuicChromiumClientStream to > > QuicSessionKey. > > Does EMU for std::map<QuicChromiumClientStream*, QuicSessionKey> already taken > > into account the size of QuicSessionKey? > > So, the convention is that EMU(T) returns "additional" memory beyond the > sizeof(T). Classes that contain QuicSessionKey will account for its sizeof(). > For example EMU(unique_ptr<T>) is sizeof(T) + EMU(T). > > The easiest is just to call EMU() on all members that can allocate, without > thinking of how big or small they can get. > > So I would include both destination_ and server_id_ here. Done. https://codereview.chromium.org/2650943007/diff/60001/net/quic/chromium/quic_... File net/quic/chromium/quic_chromium_client_session.cc (right): https://codereview.chromium.org/2650943007/diff/60001/net/quic/chromium/quic_... net/quic/chromium/quic_chromium_client_session.cc:1525: // QuicHeaderList and QuicSession's QuiCWriteBlockedList. On 2017/01/25 23:52:00, Ryan Hamilton wrote: > Would you also want to estimate any open streams? Would you also want to > estimate the memory usage from the unacked packet map? (packets which have been > sent but not received, in which case we need to keep the data around in case we > need to retransmit it) Done. Added in the TODO. I would like to estimate all non-trivial objects. Open streams are a good place to do next. Thanks for the suggestions. My main focus right now is to investigate where //net memory is going when there are no active requests. The next thing to investigate the "active requests" case. One challenge I have with instrumenting QUIC code is that I might need to touch /quic/core. Ryan: how do you feel about I land the change in chromium first and do an upstream CL shortly after? Or do you prefer I go the other way? https://codereview.chromium.org/2650943007/diff/100001/net/base/host_port_pai... File net/base/host_port_pair.cc (right): https://codereview.chromium.org/2650943007/diff/100001/net/base/host_port_pai... net/base/host_port_pair.cc:79: return base::trace_event::EstimateMemoryUsage(host_) + sizeof(port_); Dmitry: EMU doesn't have a version for primitives. Is using sizeof(port_) here appropriate? https://codereview.chromium.org/2650943007/diff/100001/net/quic/core/quic_ser... File net/quic/core/quic_server_id.cc (right): https://codereview.chromium.org/2650943007/diff/100001/net/quic/core/quic_ser... net/quic/core/quic_server_id.cc:60: // FIXME: Ryan: can i depend on base::trace_event in quic/core ? Ryan: can I depend on base/trace_event for files in quic/core? Is the dependency allowed?
lgtm https://codereview.chromium.org/2650943007/diff/60001/net/quic/chromium/quic_... File net/quic/chromium/quic_chromium_client_session.cc (right): https://codereview.chromium.org/2650943007/diff/60001/net/quic/chromium/quic_... net/quic/chromium/quic_chromium_client_session.cc:1525: // QuicHeaderList and QuicSession's QuiCWriteBlockedList. On 2017/01/26 15:23:50, xunjieli wrote: > On 2017/01/25 23:52:00, Ryan Hamilton wrote: > > Would you also want to estimate any open streams? Would you also want to > > estimate the memory usage from the unacked packet map? (packets which have > been > > sent but not received, in which case we need to keep the data around in case > we > > need to retransmit it) > > Done. Added in the TODO. > I would like to estimate all non-trivial objects. Open streams are a good place > to do next. Thanks for the suggestions. My main focus right now is to > investigate where //net memory is going when there are no active requests. The > next thing to investigate the "active requests" case. > > One challenge I have with instrumenting QUIC code is that I might need to touch > /quic/core. > > Ryan: how do you feel about I land the change in chromium first and do an > upstream CL shortly after? Or do you prefer I go the other way? Both can work, but it's usually better to land internally first. However, sometimes I'll do this by: 1) Write the CL in chrome. *iterate* 2) When finished, make the changes in an internal CL and send that for review. 3) When it lands, marge any changes from the review back to your chrome CL and land that. https://codereview.chromium.org/2650943007/diff/100001/net/quic/core/quic_ser... File net/quic/core/quic_server_id.cc (right): https://codereview.chromium.org/2650943007/diff/100001/net/quic/core/quic_ser... net/quic/core/quic_server_id.cc:60: // FIXME: Ryan: can i depend on base::trace_event in quic/core ? On 2017/01/26 15:23:51, xunjieli wrote: > Ryan: can I depend on base/trace_event for files in quic/core? > Is the dependency allowed? No, that's not allowed, since base/trace_event does not exist internally. Hopefully that's not too big of an issue? Perhaps we can encapsulate the differences in some way.
https://codereview.chromium.org/2650943007/diff/100001/net/base/host_port_pai... File net/base/host_port_pair.cc (right): https://codereview.chromium.org/2650943007/diff/100001/net/base/host_port_pai... net/base/host_port_pair.cc:79: return base::trace_event::EstimateMemoryUsage(host_) + sizeof(port_); On 2017/01/26 15:23:51, xunjieli wrote: > Dmitry: EMU doesn't have a version for primitives. Is using sizeof(port_) here > appropriate? It should be just EMU(host_), because sizeof(port) is not really an allocation. Actually, deep down (i.e. not a public API) there is a version of EMU() for integers etc. that just returns 0 (to support things like std::map<int, std::string>). EMU(std::string) doesn't include sizeof(std::string) either, it includes dynamic memory that std::string allocates. Here is how sizeof(port) (and other static overhead) is accounted: 1. sizeof(port) is contained in sizeof(HostPortPair) 2. sizeof(HostPortPair) is contained in sizeof(QuicSessionKey) 3. which is in turn contained in std::pair<QuicChromiumClientStream*, QuicSessionKey> 4. which is contained in tree Node (see EstimateTreeMemoryUsage in memory_usage_estimator.h) 5. and it's the tree Node which is accounted for (sizeof(Node) * count) So, all static overhead is accounted by some root-level estimator, and not needed to be accounted in EMUs.
Thanks! Since this contains one upstream file, I will land the change in internal repo. Once that's done, I will land this one if no one objects. https://codereview.chromium.org/2650943007/diff/60001/net/quic/chromium/quic_... File net/quic/chromium/quic_chromium_client_session.cc (right): https://codereview.chromium.org/2650943007/diff/60001/net/quic/chromium/quic_... net/quic/chromium/quic_chromium_client_session.cc:1525: // QuicHeaderList and QuicSession's QuiCWriteBlockedList. On 2017/01/26 17:04:23, Ryan Hamilton wrote: > On 2017/01/26 15:23:50, xunjieli wrote: > > On 2017/01/25 23:52:00, Ryan Hamilton wrote: > > > Would you also want to estimate any open streams? Would you also want to > > > estimate the memory usage from the unacked packet map? (packets which have > > been > > > sent but not received, in which case we need to keep the data around in case > > we > > > need to retransmit it) > > > > Done. Added in the TODO. > > I would like to estimate all non-trivial objects. Open streams are a good > place > > to do next. Thanks for the suggestions. My main focus right now is to > > investigate where //net memory is going when there are no active requests. > The > > next thing to investigate the "active requests" case. > > > > One challenge I have with instrumenting QUIC code is that I might need to > touch > > /quic/core. > > > > Ryan: how do you feel about I land the change in chromium first and do an > > upstream CL shortly after? Or do you prefer I go the other way? > > Both can work, but it's usually better to land internally first. However, > sometimes I'll do this by: > 1) Write the CL in chrome. *iterate* > 2) When finished, make the changes in an internal CL and send that for review. > 3) When it lands, marge any changes from the review back to your chrome CL and > land that. Acknowledged. Got it. Will do that. https://codereview.chromium.org/2650943007/diff/100001/net/base/host_port_pai... File net/base/host_port_pair.cc (right): https://codereview.chromium.org/2650943007/diff/100001/net/base/host_port_pai... net/base/host_port_pair.cc:79: return base::trace_event::EstimateMemoryUsage(host_) + sizeof(port_); On 2017/01/26 17:19:35, DmitrySkiba wrote: > On 2017/01/26 15:23:51, xunjieli wrote: > > Dmitry: EMU doesn't have a version for primitives. Is using sizeof(port_) here > > appropriate? > > It should be just EMU(host_), because sizeof(port) is not really an allocation. > Actually, deep down (i.e. not a public API) there is a version of EMU() for > integers etc. that just returns 0 (to support things like std::map<int, > std::string>). > > EMU(std::string) doesn't include sizeof(std::string) either, it includes dynamic > memory that std::string allocates. > > Here is how sizeof(port) (and other static overhead) is accounted: > 1. sizeof(port) is contained in sizeof(HostPortPair) > 2. sizeof(HostPortPair) is contained in sizeof(QuicSessionKey) > 3. which is in turn contained in std::pair<QuicChromiumClientStream*, > QuicSessionKey> > 4. which is contained in tree Node (see EstimateTreeMemoryUsage in > memory_usage_estimator.h) > 5. and it's the tree Node which is accounted for (sizeof(Node) * count) > > So, all static overhead is accounted by some root-level estimator, and not > needed to be accounted in EMUs. Done. I understand to use EMU better now. Thanks a lot for the detailed explanation! https://codereview.chromium.org/2650943007/diff/100001/net/quic/core/quic_ser... File net/quic/core/quic_server_id.cc (right): https://codereview.chromium.org/2650943007/diff/100001/net/quic/core/quic_ser... net/quic/core/quic_server_id.cc:60: // FIXME: Ryan: can i depend on base::trace_event in quic/core ? On 2017/01/26 17:04:23, Ryan Hamilton wrote: > On 2017/01/26 15:23:51, xunjieli wrote: > > Ryan: can I depend on base/trace_event for files in quic/core? > > Is the dependency allowed? > > No, that's not allowed, since base/trace_event does not exist internally. > Hopefully that's not too big of an issue? Perhaps we can encapsulate the > differences in some way. Got it. I will make sure I work around that. Thanks!
lgtm https://codereview.chromium.org/2650943007/diff/140001/net/quic/chromium/quic... File net/quic/chromium/quic_chromium_client_session.cc (right): https://codereview.chromium.org/2650943007/diff/140001/net/quic/chromium/quic... net/quic/chromium/quic_chromium_client_session.cc:1526: // unacked packet map. nit: it looks like the memory usage estimation is currently only for packet reader, should we add a comment in the .h file?
Thanks! I removed the change in QuicServerId. I created a static function in host_port_pair.cc and a file-scoped function in quic_stream_factory.cc. PTAL. https://codereview.chromium.org/2650943007/diff/140001/net/quic/chromium/quic... File net/quic/chromium/quic_chromium_client_session.cc (right): https://codereview.chromium.org/2650943007/diff/140001/net/quic/chromium/quic... net/quic/chromium/quic_chromium_client_session.cc:1526: // unacked packet map. On 2017/01/26 20:07:58, Zhongyi Shi wrote: > nit: it looks like the memory usage estimation is currently only for packet > reader, should we add a comment in the .h file? Done.
one extra note. https://codereview.chromium.org/2650943007/diff/180001/net/base/host_port_pair.h File net/base/host_port_pair.h (right): https://codereview.chromium.org/2650943007/diff/180001/net/base/host_port_pai... net/base/host_port_pair.h:38: static size_t EstimateMemoryUsage(const HostPortPair& pair); If this is a problem, I can put it somewhere else in net/. base/ can't depend on net/, so i can't move it to base/trace_event/
https://codereview.chromium.org/2650943007/diff/200001/net/quic/chromium/quic... File net/quic/chromium/quic_stream_factory.cc (right): https://codereview.chromium.org/2650943007/diff/200001/net/quic/chromium/quic... net/quic/chromium/quic_stream_factory.cc:1078: EstimateServerIdMemoryUsage(server_id_); I think this should just return 0 here instead of trying to estimate it. I actually looked at the string from running chrome and it is 25bytes large. Even if we have 1000 QuicSessionKey objects around, we will still see 20KiB in total. I do not think the instrumentation discussion is worth the time given the marginal improvement in accuracy of the measurement. I have tried opening a lot of tabs and still don't see more than 25 instances of the QuicSessionKey at any time. I think if we are going to show 100 bytes after adding these 3 functions along with a downstream change, it just feels useless.
https://codereview.chromium.org/2650943007/diff/200001/net/quic/chromium/quic... File net/quic/chromium/quic_stream_factory.cc (right): https://codereview.chromium.org/2650943007/diff/200001/net/quic/chromium/quic... net/quic/chromium/quic_stream_factory.cc:1078: EstimateServerIdMemoryUsage(server_id_); On 2017/01/26 22:43:59, ssid wrote: > I think this should just return 0 here instead of trying to estimate it. I > actually looked at the string from running chrome and it is 25bytes large. Even > if we have 1000 QuicSessionKey objects around, we will still see 20KiB in total. > I do not think the instrumentation discussion is worth the time given the > marginal improvement in accuracy of the measurement. I have tried opening a lot > of tabs and still don't see more than 25 instances of the QuicSessionKey at any > time. > > I think if we are going to show 100 bytes after adding these 3 functions along > with a downstream change, it just feels useless. Dmitry suggested to track all allocations regardless size. But I agree with you that adding 2 extra methods here probably outweighs the benefits. Dmitry do you feel strongly about this? Should we go back to return 0 for this one?
On 2017/01/26 23:10:00, xunjieli wrote: > https://codereview.chromium.org/2650943007/diff/200001/net/quic/chromium/quic... > File net/quic/chromium/quic_stream_factory.cc (right): > > https://codereview.chromium.org/2650943007/diff/200001/net/quic/chromium/quic... > net/quic/chromium/quic_stream_factory.cc:1078: > EstimateServerIdMemoryUsage(server_id_); > On 2017/01/26 22:43:59, ssid wrote: > > I think this should just return 0 here instead of trying to estimate it. I > > actually looked at the string from running chrome and it is 25bytes large. > Even > > if we have 1000 QuicSessionKey objects around, we will still see 20KiB in > total. > > I do not think the instrumentation discussion is worth the time given the > > marginal improvement in accuracy of the measurement. I have tried opening a > lot > > of tabs and still don't see more than 25 instances of the QuicSessionKey at > any > > time. > > > > I think if we are going to show 100 bytes after adding these 3 functions along > > with a downstream change, it just feels useless. > > Dmitry suggested to track all allocations regardless size. But I agree with you > that adding 2 extra methods here probably outweighs the benefits. Dmitry do you > feel strongly about this? Should we go back to return 0 for this one? I think we should still estimate it, just for completeness sake. It's easier to just dump everything than to think about it, measure, reason about it, etc. Besides, I don't see a problem in measuring it, there are no complex computations there.
The CQ bit was checked by xunjieli@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Thanks everyone. I am going to land this as it is. Please feel free to add more comments -- I will address them separately if needed.
The CQ bit was checked by xunjieli@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from ssid@chromium.org, rch@chromium.org, zhongyi@chromium.org Link to the patchset: https://codereview.chromium.org/2650943007/#ps200001 (title: "be more specific in comment")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 200001, "attempt_start_ts": 1485790317957130,
"parent_rev": "9255195315efa671012f0bd0da1afe685d70b2f6", "commit_rev":
"69720dd78be79fe965b684ef4a753e3947d20317"}
Message was sent while issue was closed.
Description was changed from ========== Track QuicChromiumClientSession in net/ MemoryDumpProvider This CL tracks the number of QuicChromiumClientSession in net/ MemoryDumpProvider. Follow-up CLs will improve the coverage of the memory usage estimate. BUG=669108 ========== to ========== Track QuicChromiumClientSession in net/ MemoryDumpProvider This CL tracks the number of QuicChromiumClientSession in net/ MemoryDumpProvider. Follow-up CLs will improve the coverage of the memory usage estimate. BUG=669108 Review-Url: https://codereview.chromium.org/2650943007 Cr-Commit-Position: refs/heads/master@{#446986} Committed: https://chromium.googlesource.com/chromium/src/+/69720dd78be79fe965b684ef4a75... ==========
Message was sent while issue was closed.
Committed patchset #9 (id:200001) as https://chromium.googlesource.com/chromium/src/+/69720dd78be79fe965b684ef4a75... |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
