[Autofill] Discard unused FormStructure in AutofillManager (avoid possible high memory usage) |
|||||||||||||||||
Issue descriptionThe desktop memory team is profiling the browser process by recording all allocations, and tracking live allocations. Reports are automatically uploaded when memory usage crosses a threshold. In this case, we received a report of a browser process that was using 310MB of malloc on macOS. go/crash/ffd15e77c368dde7. I've attached a json file that shows all the major allocations in the browser process. To view the json file, copy its contents to http://jsonviewer.stack.hu/. The three stacks I've attached as screenshots are responsible for ~50k live objects in the browser process, that account for 10MB of memory. This seems like an exceptional amount for Autofill to use, and may indicate the presence of a live leak [memory that is still reachable, but is unused]. Glancing through AutofillManager::ParseForm, it looks like form_structures_ will continually grow until Reset() is called, which happens on main frame navigation. But this means that if a user has a long-running page that generates/destroy many forms...then this list will potentially grow indefinitely? Please help us better understand what's going on here. Note that ~85% of the bugs we've filed with this tool have resulted in the discovery of real issues, so please don't just dismiss this out of hand. :) + autofill OWNERs + ssid: Does this code affect Android?
,
Jan 16 2018
,
May 1 2018
,
May 15 2018
,
Aug 15
,
Aug 15
,
Aug 18
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/008b3ce56694851ea6d0a6bcdbe961d878cd8b99 commit 008b3ce56694851ea6d0a6bcdbe961d878cd8b99 Author: Roger McFarlane <rogerm@chromium.org> Date: Sat Aug 18 09:59:36 2018 [autofill] Refactor form structure cache from vector to map. The current implementation uses a vector to store all the forms that we parse. The code to find cached forms the searches that vector starting from the back. This CL changes the cache to a map instead, so each form will be saved only once. The key is the FormSignature and the value is a unique ptr to the FormStructure. Notable differences - AutofillHandler now holds a map, by form signature, of FormStructure unique_ptrs instead of a vector. - Parsing/updating FormData associated with a given signature now delete any previously cached FormStructure for that signature. - form_structure_browsertests sorts currently known form structures by their earliest parse time, to preserve the output ordering. - AutofillHandler::FindCachedForm To update an existing form signature, this function is O(lg n). when inserting a new form signaure this function is O(n + log n). Bug: 769617, 864469, 864503, 865640, 866504 Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet Change-Id: I630336d6672a57acbe0257501c44f540b39341de Reviewed-on: https://chromium-review.googlesource.com/1175456 Commit-Queue: Roger McFarlane <rogerm@chromium.org> Reviewed-by: Moe Ahmadi <mahmadi@chromium.org> Cr-Commit-Position: refs/heads/master@{#584311} [modify] https://crrev.com/008b3ce56694851ea6d0a6bcdbe961d878cd8b99/chrome/browser/autofill/form_structure_browsertest.cc [modify] https://crrev.com/008b3ce56694851ea6d0a6bcdbe961d878cd8b99/components/autofill/core/browser/autofill_assistant.cc [modify] https://crrev.com/008b3ce56694851ea6d0a6bcdbe961d878cd8b99/components/autofill/core/browser/autofill_assistant.h [modify] https://crrev.com/008b3ce56694851ea6d0a6bcdbe961d878cd8b99/components/autofill/core/browser/autofill_assistant_unittest.cc [modify] https://crrev.com/008b3ce56694851ea6d0a6bcdbe961d878cd8b99/components/autofill/core/browser/autofill_handler.cc [modify] https://crrev.com/008b3ce56694851ea6d0a6bcdbe961d878cd8b99/components/autofill/core/browser/autofill_handler.h [modify] https://crrev.com/008b3ce56694851ea6d0a6bcdbe961d878cd8b99/components/autofill/core/browser/autofill_manager.cc [modify] https://crrev.com/008b3ce56694851ea6d0a6bcdbe961d878cd8b99/components/autofill/core/browser/autofill_manager.h [modify] https://crrev.com/008b3ce56694851ea6d0a6bcdbe961d878cd8b99/components/autofill/core/browser/autofill_manager_unittest.cc [modify] https://crrev.com/008b3ce56694851ea6d0a6bcdbe961d878cd8b99/components/autofill/core/browser/autofill_metrics_unittest.cc [modify] https://crrev.com/008b3ce56694851ea6d0a6bcdbe961d878cd8b99/components/autofill/core/browser/autofill_test_utils.cc [modify] https://crrev.com/008b3ce56694851ea6d0a6bcdbe961d878cd8b99/components/autofill/core/browser/autofill_test_utils.h [modify] https://crrev.com/008b3ce56694851ea6d0a6bcdbe961d878cd8b99/components/autofill/core/browser/credit_card_save_manager_unittest.cc [modify] https://crrev.com/008b3ce56694851ea6d0a6bcdbe961d878cd8b99/components/autofill/core/browser/form_structure.cc [modify] https://crrev.com/008b3ce56694851ea6d0a6bcdbe961d878cd8b99/components/autofill/core/browser/form_structure.h [modify] https://crrev.com/008b3ce56694851ea6d0a6bcdbe961d878cd8b99/components/autofill/core/browser/test_autofill_manager.cc [modify] https://crrev.com/008b3ce56694851ea6d0a6bcdbe961d878cd8b99/ios/chrome/browser/autofill/autofill_controller_unittest.mm [modify] https://crrev.com/008b3ce56694851ea6d0a6bcdbe961d878cd8b99/ios/chrome/browser/autofill/form_structure_browsertest.mm
,
Nov 29
vabr going hobby only -> reducing involvement. Please contact me directly in urgent matters. |
|||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||
Comment 1 by ma...@chromium.org
, Sep 28 2017Labels: -Pri-3 Pri-2
Owner: rogerm@chromium.org
Status: Assigned (was: Untriaged)
Summary: [Autofill] Discard unused FormStructure in AutofillManager (avoid possible high memory usage) (was: Autofill uses 10MB in browser process.)