diff options
| -rw-r--r-- | base/process/process_metrics.h | 3 | ||||
| -rw-r--r-- | base/process/process_metrics_win.cc | 2 | ||||
| -rw-r--r-- | chrome/browser/private_working_set_snapshot.h | 110 | ||||
| -rw-r--r-- | chrome/browser/private_working_set_snapshot_win.cc | 160 | ||||
| -rw-r--r-- | chrome/browser/private_working_set_snapshot_win_unittest.cc | 52 | ||||
| -rw-r--r-- | chrome/browser/task_manager/task_manager.cc | 45 | ||||
| -rw-r--r-- | chrome/browser/task_manager/task_manager.h | 9 | ||||
| -rw-r--r-- | chrome/chrome_browser.gypi | 2 | ||||
| -rw-r--r-- | chrome/chrome_tests_unit.gypi | 1 |
9 files changed, 6 insertions, 378 deletions
diff --git a/base/process/process_metrics.h b/base/process/process_metrics.h index 8b4ec86..5916b94 100644 --- a/base/process/process_metrics.h +++ b/base/process/process_metrics.h @@ -140,8 +140,7 @@ class BASE_EXPORT ProcessMetrics { // memory usage as per definition of CommittedBytes. void GetCommittedKBytes(CommittedKBytes* usage) const; // Fills a WorkingSetKBytes containing resident private and shared memory - // usage in bytes, as per definition of WorkingSetBytes. Note that this - // function is somewhat expensive on Windows (a few ms per process). + // usage in bytes, as per definition of WorkingSetBytes. bool GetWorkingSetKBytes(WorkingSetKBytes* ws_usage) const; #if defined(OS_MACOSX) diff --git a/base/process/process_metrics_win.cc b/base/process/process_metrics_win.cc index 170f6dc..6c424e3 100644 --- a/base/process/process_metrics_win.cc +++ b/base/process/process_metrics_win.cc @@ -260,7 +260,7 @@ static BOOL InternalGetPerformanceInfo( GetProcAddress(psapi_dll, "GetPerformanceInfo")); if (!GetPerformanceInfo_func) { - // The function could not be loaded! + // The function could be loaded! memset(pPerformanceInformation, 0, cb); return FALSE; } diff --git a/chrome/browser/private_working_set_snapshot.h b/chrome/browser/private_working_set_snapshot.h deleted file mode 100644 index e7d0910..0000000 --- a/chrome/browser/private_working_set_snapshot.h +++ /dev/null @@ -1,110 +0,0 @@ -// Copyright 2015 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#ifndef CHROME_BROWSER_PRIVATE_WORKING_SET_SNAPSHOT_H_ -#define CHROME_BROWSER_PRIVATE_WORKING_SET_SNAPSHOT_H_ - -#include "build/build_config.h" - -#if defined(OS_WIN) -#include <pdh.h> - -#include <vector> - -#include "base/process/process_handle.h" -#include "base/win/scoped_handle.h" - -namespace win { - -// The traits class for PDH handles that can be closed via PdhCloseQuery() API. -struct PDHHandleTraits { - typedef PDH_HQUERY Handle; - static PDH_HQUERY NullHandle() { return nullptr; } - static bool IsHandleValid(PDH_HQUERY handle) { return handle != nullptr; } - static bool CloseHandle(PDH_HQUERY handle) { - return (PdhCloseQuery(handle) == ERROR_SUCCESS); - } - - private: - DISALLOW_IMPLICIT_CONSTRUCTORS(PDHHandleTraits); -}; - -// DummyVerifierTraits can be used because PDH_HQUERY is just a typedef for -// HANDLE. However HandleTraits cannot be used because PdhCloseQuery must be -// called rather than CloseHandle to dispose of the resources. -using ScopedPDH = - base::win::GenericScopedHandle<PDHHandleTraits, - base::win::DummyVerifierTraits>; - -} // namespace win - -// This class can be used to do bulk collection of private working sets. This -// exists because on Windows it is much faster to collect a group of private -// working sets all at once using PdhOpenQuery than to calculate the private -// working sets for each process individually. -class PrivateWorkingSetSnapshot { - public: - PrivateWorkingSetSnapshot(); - ~PrivateWorkingSetSnapshot(); - - // Add a process name that this object should monitor, such as "chrome". All - // processes whose name starts with this string will be monitored. - void AddToMonitorList(const std::string& process_name); - - // Query the system for working-set information for all monitored processes - // and update the results cache. This function may take a few ms to run. - // The time it takes seems to be independent of the number of processes it - // retrieves data for. This makes it faster than using QueryWorkingSet as soon - // as the process count exceeds two or three. - void Sample(); - - // Ask for the working set for a specific process, from the most recent call - // to Sample. If no data is available then zero will be returned. The result - // is in bytes. - size_t GetPrivateWorkingSet(base::ProcessId process_id) const; - - private: - // This holds a pair of Pdh counters to queries for the process ID and private - // working set for a particular process name being monitored. The results from - // the two queries can be matched up so that we can associate a private - // working set with a process ID. - struct PdhCounterPair { - // These are bound to query_handle_ and will be freed when it is closed. - // The handle to the 'counter' that retrieves process IDs. - PDH_HCOUNTER process_id_handle = nullptr; - // The handle to the 'counter' that retrieves private working sets. - PDH_HCOUNTER private_ws_handle = nullptr; - }; - - // Struct for storing a process ID and associated private working set. - struct PidAndPrivateWorkingSet { - base::ProcessId process_id; - size_t private_ws; - // Comparison function for sorting by process ID. - bool operator<(const PidAndPrivateWorkingSet& other) const { - // private_ws is intentionally *not* part of the comparison because it is - // the payload and process_id is the key. - return process_id < other.process_id; - } - // Comparison function for searching by process ID. - bool operator<(const base::ProcessId other_process_id) const { - return process_id < other_process_id; - } - }; - - // The handle to the query object. - win::ScopedPDH query_handle_; - - // A PdhCounterPair for each successful AddToMonitorList call. - std::vector<PdhCounterPair> counter_pairs_; - - // After each call to Sample this will hold the results, sorted by process id. - std::vector<PidAndPrivateWorkingSet> records_; - - DISALLOW_COPY_AND_ASSIGN(PrivateWorkingSetSnapshot); -}; - -#endif // defined(OS_WIN) - -#endif // CHROME_BROWSER_PRIVATE_WORKING_SET_SNAPSHOT_H_ diff --git a/chrome/browser/private_working_set_snapshot_win.cc b/chrome/browser/private_working_set_snapshot_win.cc deleted file mode 100644 index 56fb568..0000000 --- a/chrome/browser/private_working_set_snapshot_win.cc +++ /dev/null @@ -1,160 +0,0 @@ -// Copyright 2015 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#include "chrome/browser/private_working_set_snapshot.h" - -#include <pdh.h> -#include <pdhmsg.h> - -#include <algorithm> - -#include "base/numerics/safe_conversions.h" -#include "base/win/windows_version.h" - -// Link pdh.lib (import library for pdh.dll) whenever this file is linked in. -#pragma comment(lib, "pdh.lib") - -PrivateWorkingSetSnapshot::PrivateWorkingSetSnapshot() { - // The Pdh APIs are supported on Windows XP, but the "Working Set - Private" - // counter that PrivateWorkingSetSnapshot depends on is not defined until - // Windows Vista. Early-out to avoid wasted effort. All queries will return - // zero and will have to use the fallback calculations. - if (base::win::GetVersion() < base::win::VERSION_VISTA) - return; - - // Create a Pdh query - PDH_HQUERY query_handle; - if (PdhOpenQuery(NULL, NULL, &query_handle) != ERROR_SUCCESS) { - return; - } - - query_handle_.Set(query_handle); -} - -PrivateWorkingSetSnapshot::~PrivateWorkingSetSnapshot() { -} - -void PrivateWorkingSetSnapshot::AddToMonitorList( - const std::string& process_name) { - if (!query_handle_.IsValid()) - return; - - // Create the magic strings that will return a list of process IDs and a list - // of private working sets. The 'process_name' variable should be something - // like "chrome". The '*' character indicates that we want records for all - // processes whose names start with process_name - all chrome processes, but - // also all 'chrome_editor.exe' processes or other matching names. The excess - // information is unavoidable but harmless. - std::string process_id_query = "\\Process(" + process_name + "*)\\ID Process"; - std::string private_ws_query = - "\\Process(" + process_name + "*)\\Working Set - Private"; - - // Add the two counters to the query. - PdhCounterPair new_counters; - if (PdhAddCounterA(query_handle_.Get(), process_id_query.c_str(), NULL, - &new_counters.process_id_handle) != ERROR_SUCCESS) { - return; - } - - // If adding the second counter fails then we should remove the first one. - if (PdhAddCounterA(query_handle_.Get(), private_ws_query.c_str(), NULL, - &new_counters.private_ws_handle) != ERROR_SUCCESS) { - PdhRemoveCounter(new_counters.process_id_handle); - } - - // Record the pair of counter query handles so that we can query them later. - counter_pairs_.push_back(new_counters); -} - -void PrivateWorkingSetSnapshot::Sample() { - if (counter_pairs_.empty()) - return; - - // Destroy all previous data. - records_.resize(0); - // Record the requested data into PDH's internal buffers. - if (PdhCollectQueryData(query_handle_.Get()) != ERROR_SUCCESS) - return; - - for (auto& counter_pair : counter_pairs_) { - // Find out how much space is required for the two counter arrays. - // A return code of PDH_MORE_DATA indicates that we should call again with - // the buffer size returned. - DWORD buffer_size1 = 0; - DWORD item_count1 = 0; - // Process IDs should be retrieved as PDH_FMT_LONG - if (PdhGetFormattedCounterArray(counter_pair.process_id_handle, - PDH_FMT_LONG, &buffer_size1, &item_count1, - nullptr) != PDH_MORE_DATA) - continue; - if (buffer_size1 == 0 || item_count1 == 0) - continue; - - DWORD buffer_size2 = 0; - DWORD item_count2 = 0; - // Working sets should be retrieved as PDH_FMT_LARGE (LONGLONG) - // Note that if this second call to PdhGetFormattedCounterArray with the - // buffer size and count variables being zero is omitted then the PID and - // working-set results are not reliably correlated. - if (PdhGetFormattedCounterArray(counter_pair.private_ws_handle, - PDH_FMT_LARGE, &buffer_size2, &item_count2, - nullptr) != PDH_MORE_DATA) - continue; - - // It is not clear whether Pdh guarantees that the two counters in the same - // query will execute atomically - if they will see the same set of - // processes. If they do not then the correspondence between "ID Process" - // and "Working Set - Private" is lost and we have to discard these results. - // In testing these values have always matched. If this check fails then - // the old per-process memory calculations will be used instead. - if (buffer_size1 != buffer_size2 || item_count1 != item_count2) - continue; - - // Allocate enough space for the results of both queries. - std::vector<char> buffer(buffer_size1 * 2); - // Retrieve the process ID data. - auto process_id_data = - reinterpret_cast<PDH_FMT_COUNTERVALUE_ITEM*>(&buffer[0]); - if (PdhGetFormattedCounterArray(counter_pair.process_id_handle, - PDH_FMT_LONG, &buffer_size1, &item_count1, - process_id_data) != ERROR_SUCCESS) - continue; - // Retrieve the private working set data. - auto private_ws_data = - reinterpret_cast<PDH_FMT_COUNTERVALUE_ITEM*>(&buffer[buffer_size1]); - if (PdhGetFormattedCounterArray(counter_pair.private_ws_handle, - PDH_FMT_LARGE, &buffer_size1, &item_count1, - private_ws_data) != ERROR_SUCCESS) - continue; - - // Make room for the new set of records. - size_t start_offset = records_.size(); - records_.resize(start_offset + item_count1); - - for (DWORD i = 0; i < item_count1; ++i) { - records_[start_offset + i].process_id = - process_id_data[i].FmtValue.longValue; - // Integer overflow can happen here if a 32-bit process is monitoring a - // 64-bit process so we do a saturated_cast. - records_[start_offset + i].private_ws = - base::saturated_cast<size_t>(private_ws_data[i].FmtValue.largeValue); - } - } - - // The results will include all processes that match the passed in name, - // regardless of whether they are spawned by the calling process. - // The results must be sorted by process ID for efficient lookup. - std::sort(records_.begin(), records_.end()); -} - -size_t PrivateWorkingSetSnapshot::GetPrivateWorkingSet( - base::ProcessId process_id) const { - // Do a binary search for the requested process ID and return the working set - // if found. - auto p = std::lower_bound(records_.begin(), records_.end(), process_id); - if (p != records_.end() && p->process_id == process_id) - return p->private_ws; - - return 0; -} diff --git a/chrome/browser/private_working_set_snapshot_win_unittest.cc b/chrome/browser/private_working_set_snapshot_win_unittest.cc deleted file mode 100644 index f77f034..0000000 --- a/chrome/browser/private_working_set_snapshot_win_unittest.cc +++ /dev/null @@ -1,52 +0,0 @@ -// Copyright 2015 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#include "chrome/browser/private_working_set_snapshot.h" - -#include "base/win/windows_version.h" -#include "testing/gtest/include/gtest/gtest.h" - -using PrivateWorkingSetSnapshotWinTest = testing::Test; - -TEST_F(PrivateWorkingSetSnapshotWinTest, FindPidSelfTest) { - // The Pdh APIs are supported on Windows XP, but the "Working Set - Private" - // counter that PrivateWorkingSetSnapshot depends on is not defined until - // Windows Vista. Early-out to avoid test failure. - if (base::win::GetVersion() < base::win::VERSION_VISTA) - return; - - // Sample this process. - base::ProcessId pid = base::GetCurrentProcId(); - - PrivateWorkingSetSnapshot private_ws_snapshot; - - private_ws_snapshot.AddToMonitorList("unit_tests"); - private_ws_snapshot.Sample(); - - size_t private_ws = private_ws_snapshot.GetPrivateWorkingSet(pid); - // Private working set is difficult to predict but should be at least several - // MB. Initial tests show a value of 19+ MB depending on how many tests and - // processes are used. Anomalously small or large values would warrant - // investigation. - EXPECT_GT(private_ws, 2000000u); - // Check that the WS is less than 500 MB. This is set very high to reduce the - // chance that unrelated changes could ever make this fail. This mostly just - // checks against some uncaught error that might return 0xFFFFFFFF. - EXPECT_LT(private_ws, 500000000u); - - // Allocate and touch a large block of memory (vector's constructor will zero - // every entry). This will increase the private working set. - const size_t alloc_size = 10000000; - std::vector<char> big_memory(alloc_size); - - size_t private_ws2 = private_ws_snapshot.GetPrivateWorkingSet(pid); - EXPECT_EQ(private_ws, private_ws2) << "GetPrivateWorkingSet should be " - "consistent until the next call to " - "Sample()"; - - private_ws_snapshot.Sample(); - size_t private_ws3 = private_ws_snapshot.GetPrivateWorkingSet(pid); - EXPECT_GT(private_ws3, private_ws2 + alloc_size / 2) - << "GetPrivateWorkingSet should increase as we allocate more memory"; -} diff --git a/chrome/browser/task_manager/task_manager.cc b/chrome/browser/task_manager/task_manager.cc index dffc79f..11d58fc 100644 --- a/chrome/browser/task_manager/task_manager.cc +++ b/chrome/browser/task_manager/task_manager.cc @@ -10,6 +10,7 @@ #include "base/location.h" #include "base/prefs/pref_registry_simple.h" #include "base/prefs/pref_service.h" +#include "base/process/process_metrics.h" #include "base/single_thread_task_runner.h" #include "base/stl_util.h" #include "base/strings/string16.h" @@ -18,7 +19,6 @@ #include "base/strings/utf_string_conversions.h" #include "base/thread_task_runner_handle.h" #include "chrome/browser/browser_process.h" -#include "chrome/browser/private_working_set_snapshot.h" #include "chrome/browser/profiles/profile_manager.h" #include "chrome/browser/profiles/profile_window.h" #include "chrome/browser/task_manager/background_information.h" @@ -274,11 +274,6 @@ TaskManagerModel::TaskManagerModel(TaskManager* task_manager) task_manager, scoped_ptr<WebContentsInformation>( new task_manager::GuestInformation()))); -#if defined(OS_WIN) - working_set_snapshot_.reset(new PrivateWorkingSetSnapshot); - working_set_snapshot_->AddToMonitorList("chrome"); - working_set_snapshot_->AddToMonitorList("nacl64"); -#endif } void TaskManagerModel::AddObserver(TaskManagerModelObserver* observer) { @@ -583,13 +578,10 @@ bool TaskManagerModel::GetPhysicalMemory(int index, size_t* result) const { // On Linux private memory is also resident. Just use it. values.physical_memory = ws_usage.priv * 1024; #else - // Memory = working_set.private which is working set minus shareable. This - // avoids the unpredictable counting that occurs when calculating memory as - // working set minus shared (renderer code counted when one tab is open and - // not counted when two or more are open) and it is much more efficient to - // calculate on Windows. + // Memory = working_set.private + working_set.shareable. + // We exclude the shared memory. values.physical_memory = iter->second->GetWorkingSetSize(); - values.physical_memory -= ws_usage.shareable * 1024; + values.physical_memory -= ws_usage.shared * 1024; #endif } *result = values.physical_memory; @@ -1131,38 +1123,9 @@ void TaskManagerModel::ModelChanged() { FOR_EACH_OBSERVER(TaskManagerModelObserver, observer_list_, OnModelChanged()); } -void TaskManagerModel::RefreshPhysicalMemoryFromWorkingSetSnapshot() { -#if defined(OS_WIN) - // Collect working-set data for all monitored processes in one operation, to - // avoid the inefficiency of retrieving it one at a time. - working_set_snapshot_->Sample(); - - for (size_t i = 0; i < resources_.size(); ++i) { - size_t private_working_set = - working_set_snapshot_->GetPrivateWorkingSet(GetProcessId(i)); - - // If working-set data is available then use it. If not then - // GetWorkingSetKBytes will retrieve the data. This is rare except on - // Windows XP where GetWorkingSetKBytes will always be used. - if (private_working_set) { - // Fill in the cache with the retrieved private working set value. - base::ProcessHandle handle = GetResource(i)->GetProcess(); - PerProcessValues& values(per_process_cache_[handle]); - values.is_physical_memory_valid = true; - // Note that the other memory fields are *not* filled in. - values.physical_memory = private_working_set; - } - } -#else -// This is a NOP on other platforms because they can efficiently retrieve -// the private working-set data on a per-process basis. -#endif -} - void TaskManagerModel::Refresh() { per_resource_cache_.clear(); per_process_cache_.clear(); - RefreshPhysicalMemoryFromWorkingSetSnapshot(); #if !defined(DISABLE_NACL) nacl::NaClBrowser* nacl_browser = nacl::NaClBrowser::GetInstance(); diff --git a/chrome/browser/task_manager/task_manager.h b/chrome/browser/task_manager/task_manager.h index c3d57b4..a9ff4f5 100644 --- a/chrome/browser/task_manager/task_manager.h +++ b/chrome/browser/task_manager/task_manager.h @@ -21,7 +21,6 @@ #include "third_party/WebKit/public/web/WebCache.h" class PrefRegistrySimple; -class PrivateWorkingSetSnapshot; class TaskManagerModel; class TaskManagerModelGpuDataManagerObserver; @@ -285,10 +284,6 @@ class TaskManagerModel : public base::RefCountedThreadSafe<TaskManagerModel> { // Updates the values for all rows. void Refresh(); - // Do a bulk repopulation of the physical_memory data on platforms where that - // is faster. - void RefreshPhysicalMemoryFromWorkingSetSnapshot(); - void NotifyVideoMemoryUsageStats( const content::GPUVideoMemoryUsageStats& video_memory_usage_stats); @@ -534,10 +529,6 @@ class TaskManagerModel : public base::RefCountedThreadSafe<TaskManagerModel> { std::vector<base::Closure> on_data_ready_callbacks_; -#if defined(OS_WIN) - scoped_ptr<PrivateWorkingSetSnapshot> working_set_snapshot_; -#endif - // All per-Resource values are stored here. mutable PerResourceCache per_resource_cache_; diff --git a/chrome/chrome_browser.gypi b/chrome/chrome_browser.gypi index 3f969b7..43001e3 100644 --- a/chrome/chrome_browser.gypi +++ b/chrome/chrome_browser.gypi @@ -683,8 +683,6 @@ 'browser/prerender/prerender_tab_helper.h', 'browser/prerender/prerender_util.cc', 'browser/prerender/prerender_util.h', - 'browser/private_working_set_snapshot.h', - 'browser/private_working_set_snapshot_win.cc', 'browser/process_info_snapshot.h', 'browser/process_info_snapshot_mac.cc', 'browser/process_resource_usage.cc', diff --git a/chrome/chrome_tests_unit.gypi b/chrome/chrome_tests_unit.gypi index 4fad215..9849594 100644 --- a/chrome/chrome_tests_unit.gypi +++ b/chrome/chrome_tests_unit.gypi @@ -199,7 +199,6 @@ 'browser/prerender/prerender_resource_throttle_unittest.cc', 'browser/prerender/prerender_unittest.cc', 'browser/prerender/prerender_util_unittest.cc', - 'browser/private_working_set_snapshot_win_unittest.cc', 'browser/process_info_snapshot_mac_unittest.cc', 'browser/process_singleton_win_unittest.cc', 'browser/profiles/file_path_verifier_win_unittest.cc', |