Ensure there is no memory bloat when experimental hints are enabled |
|||||
Issue descriptionIn the previews hints, if a loading hint is marked as experimental, then we may still load them in the memory. However, we would not count those hints towards GetMaxPageHintsInMemoryThreshhold(). We should either not load the experimental hints in the memory, or if we do, then we should count them towards the threshold.
,
Nov 2
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b40eed3b1e84a9ae7a3e71bc26a694546a6e9775 commit b40eed3b1e84a9ae7a3e71bc26a694546a6e9775 Author: Jered Gray <jegray@google.com> Date: Fri Nov 02 00:41:27 2018 Strip disabled optimizations when parsing preview hints The logic in PreviewsHints::CreateFromConfig() has been modified so that it now strips out optimizations that are disabled and only allows one of each optimization type within a PageHint. Duplicate types encountered are also stripped. This makes it so that enabling an experimental optimization will automatically disable a subsequent optimization of the same optimization type in the same PageHint. If all optimizations for a page hint are stripped, then the page hint is also stripped. This fixes a bug related to disabled page hint optimizations being usable, as the disabled optimizations will no longer exist in the hint cache. As a result of this change, when the total page hints with resource hints exceeds the cap, the number of page hints added will always exactly match the cap. This is due to the logic for stripping hints allowing us to add hints that only include some of their page hints. A DCHECK has been added verifying that non-resource loading hint optimizations never contain resource loading hints. Additionally, the description of |whitelisted_optimizations| for the PageHint proto has been updated to match expectations that it will be provided as an ordered list. Unittests have been added to verify that the stripping is handled properly and that the number of page hints allowed stops exactly at the cap. Some issues with unittests in PreviewsOptimizationGuideUnittest using InitializeMultipleResourceLoadingHints() have also been fixed. Bug: 900725, 900733 Change-Id: Ic8d8ce2de98e4e2993cab994906c3b2e43c31a22 Reviewed-on: https://chromium-review.googlesource.com/c/1311796 Reviewed-by: Doug Arnett <dougarnett@chromium.org> Reviewed-by: Tarun Bansal <tbansal@chromium.org> Commit-Queue: Jered Gray <jegray@chromium.org> Cr-Commit-Position: refs/heads/master@{#604773} [modify] https://crrev.com/b40eed3b1e84a9ae7a3e71bc26a694546a6e9775/components/optimization_guide/proto/hints.proto [modify] https://crrev.com/b40eed3b1e84a9ae7a3e71bc26a694546a6e9775/components/previews/content/previews_hints.cc [modify] https://crrev.com/b40eed3b1e84a9ae7a3e71bc26a694546a6e9775/components/previews/content/previews_hints.h [modify] https://crrev.com/b40eed3b1e84a9ae7a3e71bc26a694546a6e9775/components/previews/content/previews_hints_unittest.cc [modify] https://crrev.com/b40eed3b1e84a9ae7a3e71bc26a694546a6e9775/components/previews/content/previews_optimization_guide.cc [modify] https://crrev.com/b40eed3b1e84a9ae7a3e71bc26a694546a6e9775/components/previews/content/previews_optimization_guide_unittest.cc
,
Nov 5
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/afae6078bb535945258432310aa485e910a8d171 commit afae6078bb535945258432310aa485e910a8d171 Author: Jered Gray <jegray@google.com> Date: Mon Nov 05 20:39:34 2018 Strip disabled optimizations when parsing preview hints Merge to release branch The logic in PreviewsHints::CreateFromConfig() has been modified so that it now strips out optimizations that are disabled and only allows one of each optimization type within a PageHint. Duplicate types encountered are also stripped. This makes it so that enabling an experimental optimization will automatically disable a subsequent optimization of the same optimization type in the same PageHint. If all optimizations for a page hint are stripped, then the page hint is also stripped. This fixes a bug related to disabled page hint optimizations being usable, as the disabled optimizations will no longer exist in the hint cache. As a result of this change, when the total page hints with resource hints exceeds the cap, the number of page hints added will always exactly match the cap. This is due to the logic for stripping hints allowing us to add hints that only include some of their page hints. A DCHECK has been added verifying that non-resource loading hint optimizations never contain resource loading hints. Additionally, the description of |whitelisted_optimizations| for the PageHint proto has been updated to match expectations that it will be provided as an ordered list. Unittests have been added to verify that the stripping is handled properly and that the number of page hints allowed stops exactly at the cap. Some issues with unittests in PreviewsOptimizationGuideUnittest using InitializeMultipleResourceLoadingHints() have also been fixed. Bug: 900725, 900733 Change-Id: Ic8d8ce2de98e4e2993cab994906c3b2e43c31a22 Reviewed-on: https://chromium-review.googlesource.com/c/1311796 Reviewed-by: Doug Arnett <dougarnett@chromium.org> Reviewed-by: Tarun Bansal <tbansal@chromium.org> Commit-Queue: Jered Gray <jegray@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#604773} Reviewed-on: https://chromium-review.googlesource.com/c/1318534 Cr-Commit-Position: refs/branch-heads/3578@{#522} Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034} [modify] https://crrev.com/afae6078bb535945258432310aa485e910a8d171/components/optimization_guide/proto/hints.proto [modify] https://crrev.com/afae6078bb535945258432310aa485e910a8d171/components/previews/content/previews_hints.cc [modify] https://crrev.com/afae6078bb535945258432310aa485e910a8d171/components/previews/content/previews_hints.h [modify] https://crrev.com/afae6078bb535945258432310aa485e910a8d171/components/previews/content/previews_hints_unittest.cc [modify] https://crrev.com/afae6078bb535945258432310aa485e910a8d171/components/previews/content/previews_optimization_guide.cc [modify] https://crrev.com/afae6078bb535945258432310aa485e910a8d171/components/previews/content/previews_optimization_guide_unittest.cc
,
Nov 5
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/afae6078bb535945258432310aa485e910a8d171 Commit: afae6078bb535945258432310aa485e910a8d171 Author: jegray@google.com Commiter: tbansal@chromium.org Date: 2018-11-05 20:39:34 +0000 UTC Strip disabled optimizations when parsing preview hints Merge to release branch The logic in PreviewsHints::CreateFromConfig() has been modified so that it now strips out optimizations that are disabled and only allows one of each optimization type within a PageHint. Duplicate types encountered are also stripped. This makes it so that enabling an experimental optimization will automatically disable a subsequent optimization of the same optimization type in the same PageHint. If all optimizations for a page hint are stripped, then the page hint is also stripped. This fixes a bug related to disabled page hint optimizations being usable, as the disabled optimizations will no longer exist in the hint cache. As a result of this change, when the total page hints with resource hints exceeds the cap, the number of page hints added will always exactly match the cap. This is due to the logic for stripping hints allowing us to add hints that only include some of their page hints. A DCHECK has been added verifying that non-resource loading hint optimizations never contain resource loading hints. Additionally, the description of |whitelisted_optimizations| for the PageHint proto has been updated to match expectations that it will be provided as an ordered list. Unittests have been added to verify that the stripping is handled properly and that the number of page hints allowed stops exactly at the cap. Some issues with unittests in PreviewsOptimizationGuideUnittest using InitializeMultipleResourceLoadingHints() have also been fixed. Bug: 900725, 900733 Change-Id: Ic8d8ce2de98e4e2993cab994906c3b2e43c31a22 Reviewed-on: https://chromium-review.googlesource.com/c/1311796 Reviewed-by: Doug Arnett <dougarnett@chromium.org> Reviewed-by: Tarun Bansal <tbansal@chromium.org> Commit-Queue: Jered Gray <jegray@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#604773} Reviewed-on: https://chromium-review.googlesource.com/c/1318534 Cr-Commit-Position: refs/branch-heads/3578@{#522} Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
,
Nov 5
,
Nov 7
|
|||||
►
Sign in to add a comment |
|||||
Comment 1 by tbansal@chromium.org
, Nov 1Status: Assigned (was: Untriaged)