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

Issue 709340 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Nov 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

Limit size of servers data persisted in memory

Project Member Reported by zhongyi@chromium.org, Apr 7 2017

Issue description

Server data persisted in disk has a limited size per https://cs.chromium.org/chromium/src/net/http/http_server_properties_manager.cc?l=44.

Disk data update is scheduled when memory data updates. We currently use unlimited MRUCache(s) to persist data in memory. When writing to disk, the current code copies the data persisted in memory, swaps memory data and the copy(in order to maintain MRU order), and iterate through the copied data, adding to a MRUCache that will be persisted to disk. 

https://cs.chromium.org/chromium/src/net/http/http_server_properties_impl.cc?l=162

The iterating is unnecessary if the memory data is limited with the same size. 
 
Cc: wangyix@chromium.org
wangyix@: you might want to take a look on this when you get to the bottom of  Issue 705029 . 
Cc: -wangyix@chromium.org
Owner: wangyix@chromium.org
Status: Assigned (was: Untriaged)
Project Member

Comment 5 by bugdroid1@chromium.org, Nov 30 2017

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

commit a71cc8fd1e67364720f0894fae565a29835378ca
Author: Yixin Wang <wangyix@chromium.org>
Date: Thu Nov 30 03:37:06 2017

Refactor HttpServerPropertiesManager::UpdatePrefsWithCache()

UpdatePrefsWithCache() formats the information in HttpServerPropertiesImpl into JSON and writes it to disk.
This code has been updated to remove unnecessary copying of several base::MRUCache instances.
The behavior of UpdatePrefsWithCache() is unchanged.

Bug:  709340 
Change-Id: I43d31f4f862f103d7aea58381346b96d9d30cef4
Reviewed-on: https://chromium-review.googlesource.com/792592
Reviewed-by: Ryan Hamilton <rch@chromium.org>
Commit-Queue: Yixin Wang <wangyix@chromium.org>
Cr-Commit-Position: refs/heads/master@{#520419}
[modify] https://crrev.com/a71cc8fd1e67364720f0894fae565a29835378ca/net/http/http_server_properties_impl.cc
[modify] https://crrev.com/a71cc8fd1e67364720f0894fae565a29835378ca/net/http/http_server_properties_impl.h
[modify] https://crrev.com/a71cc8fd1e67364720f0894fae565a29835378ca/net/http/http_server_properties_impl_unittest.cc
[modify] https://crrev.com/a71cc8fd1e67364720f0894fae565a29835378ca/net/http/http_server_properties_manager.cc
[modify] https://crrev.com/a71cc8fd1e67364720f0894fae565a29835378ca/net/http/http_server_properties_manager.h
[modify] https://crrev.com/a71cc8fd1e67364720f0894fae565a29835378ca/net/http/http_server_properties_manager_unittest.cc

Project Member

Comment 6 by bugdroid1@chromium.org, Nov 30 2017

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

commit 4a227aa26991816fa67313f1d67b492b760d3b75
Author: Yixin Wang <wangyix@chromium.org>
Date: Thu Nov 30 21:33:01 2017

Change MRUCache members of HttpServerPropertiesImpl to be limited in size.

spdy_servers_map_, alternative_service_map_, server_network_stats_map_, quic_server_info_map_, and broken_alternative_services_.recently_broken_alternative_services_ can no longer take up arbitrary amounts of memory.

Bug:  709340 
Cq-Include-Trybots: master.tryserver.chromium.android:android_cronet_tester;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: Iaab61d8d982caf83eec0a62f312d6616f0ef0469
Reviewed-on: https://chromium-review.googlesource.com/798397
Reviewed-by: Helen Li <xunjieli@chromium.org>
Reviewed-by: Ryan Hamilton <rch@chromium.org>
Commit-Queue: Yixin Wang <wangyix@chromium.org>
Cr-Commit-Position: refs/heads/master@{#520697}
[modify] https://crrev.com/4a227aa26991816fa67313f1d67b492b760d3b75/components/cronet/url_request_context_config.cc
[modify] https://crrev.com/4a227aa26991816fa67313f1d67b492b760d3b75/net/http/broken_alternative_services.cc
[modify] https://crrev.com/4a227aa26991816fa67313f1d67b492b760d3b75/net/http/broken_alternative_services_unittest.cc
[modify] https://crrev.com/4a227aa26991816fa67313f1d67b492b760d3b75/net/http/http_server_properties.h
[modify] https://crrev.com/4a227aa26991816fa67313f1d67b492b760d3b75/net/http/http_server_properties_impl.cc
[modify] https://crrev.com/4a227aa26991816fa67313f1d67b492b760d3b75/net/http/http_server_properties_impl_unittest.cc
[modify] https://crrev.com/4a227aa26991816fa67313f1d67b492b760d3b75/net/http/http_server_properties_manager.cc
[modify] https://crrev.com/4a227aa26991816fa67313f1d67b492b760d3b75/net/http/http_server_properties_manager.h
[modify] https://crrev.com/4a227aa26991816fa67313f1d67b492b760d3b75/net/http/http_server_properties_manager_unittest.cc
[modify] https://crrev.com/4a227aa26991816fa67313f1d67b492b760d3b75/net/quic/chromium/quic_stream_factory_test.cc

Status: Fixed (was: Assigned)

Sign in to add a comment