Add a test for checking objects reported by DevTools memory instrumentation not allocated by tcmall…

Add a test for checking objects reported by DevTools memory instrumentation not allocated by tcmalloc

Added DevToolsInstrumentedObjectsCheck test that checks if all objects counted by DevTools memory instrumentation are actually allocated by tcmalloc. Ideally there shouldn't be other objects but at the moment about 3% of all reported objects are not allocated by tcmalloc.

BUG=None

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=160344

[dennis_jeffrey, 2012-10-04 01:16:33]

https://codereview.chromium.org/11054014/diff/1/chrome/test/functional/devtoo...
File chrome/test/functional/devtools_instrumented_objects_check.py (right):

https://codereview.chromium.org/11054014/diff/1/chrome/test/functional/devtoo...
chrome/test/functional/devtools_instrumented_objects_check.py:42: # We are not
going to use dumped profile so set the interval big enough
add period at end of sentence

https://codereview.chromium.org/11054014/diff/1/chrome/test/functional/devtoo...
chrome/test/functional/devtools_instrumented_objects_check.py:43:
os.environ['HEAP_PROFILE_TIME_INTERVAL'] = str(60 * 60)
is there a way to turn off the profile dump rather than set it to some too-long
interval value?

https://codereview.chromium.org/11054014/diff/1/chrome/test/functional/devtoo...
File chrome/test/functional/devtools_test_base.py (right):

https://codereview.chromium.org/11054014/diff/1/chrome/test/functional/devtoo...
chrome/test/functional/devtools_test_base.py:19: """Base class for DevTools
tests that use replay server
add period at end of sentence.

https://codereview.chromium.org/11054014/diff/1/chrome/test/functional/devtoo...
chrome/test/functional/devtools_test_base.py:73: pass
maybe add a comment that says this is expected to be overridden in a subclass?

https://codereview.chromium.org/11054014/diff/1/chrome/test/pyautolib/remote_...
File chrome/test/pyautolib/remote_inspector_client.py (right):

https://codereview.chromium.org/11054014/diff/1/chrome/test/pyautolib/remote_...
chrome/test/pyautolib/remote_inspector_client.py:714: FindMemoryBlock: Given an
array of memory block names return the block.
Add the two new public methods to the docstring here.

https://codereview.chromium.org/11054014/diff/1/chrome/test/pyautolib/remote_...
chrome/test/pyautolib/remote_inspector_client.py:729: def
GetInstrumentedObjectsCount(self):
add a docstring and mention what is returned if the value isn't found.

https://codereview.chromium.org/11054014/diff/1/chrome/test/pyautolib/remote_...
chrome/test/pyautolib/remote_inspector_client.py:734: return -1
would returning None be preferable to a -1?  That way we can check for a
successful return type with the "not" operator.

https://codereview.chromium.org/11054014/diff/1/chrome/test/pyautolib/remote_...
chrome/test/pyautolib/remote_inspector_client.py:736: def
GetNumberOfInstrumentedObjectsNotInHeap(self):
add a docstring and mention what is returned if the value isn't found.

https://codereview.chromium.org/11054014/diff/1/chrome/test/pyautolib/remote_...
chrome/test/pyautolib/remote_inspector_client.py:741: return -1
same comment as line 734 above

[yurys, 2012-10-04 15:07:47]

https://codereview.chromium.org/11054014/diff/1/chrome/test/functional/devtoo...
File chrome/test/functional/devtools_instrumented_objects_check.py (right):

https://codereview.chromium.org/11054014/diff/1/chrome/test/functional/devtoo...
chrome/test/functional/devtools_instrumented_objects_check.py:42: # We are not
going to use dumped profile so set the interval big enough
On 2012/10/04 01:16:33, dennis_jeffrey wrote:
> add period at end of sentence

Done.

https://codereview.chromium.org/11054014/diff/1/chrome/test/functional/devtoo...
chrome/test/functional/devtools_instrumented_objects_check.py:43:
os.environ['HEAP_PROFILE_TIME_INTERVAL'] = str(60 * 60)
On 2012/10/04 01:16:33, dennis_jeffrey wrote:
> is there a way to turn off the profile dump rather than set it to some
too-long
> interval value?
If the environment variable is omitted then the profile will be dumped only on
the process exit so I removed this line completely. Unfortunately there is no
way to disable dumping on process exit.

https://codereview.chromium.org/11054014/diff/1/chrome/test/functional/devtoo...
File chrome/test/functional/devtools_test_base.py (right):

https://codereview.chromium.org/11054014/diff/1/chrome/test/functional/devtoo...
chrome/test/functional/devtools_test_base.py:19: """Base class for DevTools
tests that use replay server
On 2012/10/04 01:16:33, dennis_jeffrey wrote:
> add period at end of sentence.

Done.

https://codereview.chromium.org/11054014/diff/1/chrome/test/functional/devtoo...
chrome/test/functional/devtools_test_base.py:73: pass
On 2012/10/04 01:16:33, dennis_jeffrey wrote:
> maybe add a comment that says this is expected to be overridden in a subclass?

Done.

https://codereview.chromium.org/11054014/diff/1/chrome/test/pyautolib/remote_...
File chrome/test/pyautolib/remote_inspector_client.py (right):

https://codereview.chromium.org/11054014/diff/1/chrome/test/pyautolib/remote_...
chrome/test/pyautolib/remote_inspector_client.py:714: FindMemoryBlock: Given an
array of memory block names return the block.
On 2012/10/04 01:16:33, dennis_jeffrey wrote:
> Add the two new public methods to the docstring here.

Done.

https://codereview.chromium.org/11054014/diff/1/chrome/test/pyautolib/remote_...
chrome/test/pyautolib/remote_inspector_client.py:729: def
GetInstrumentedObjectsCount(self):
On 2012/10/04 01:16:33, dennis_jeffrey wrote:
> add a docstring and mention what is returned if the value isn't found.

Done.

https://codereview.chromium.org/11054014/diff/1/chrome/test/pyautolib/remote_...
chrome/test/pyautolib/remote_inspector_client.py:734: return -1
On 2012/10/04 01:16:33, dennis_jeffrey wrote:
> would returning None be preferable to a -1?  That way we can check for a
> successful return type with the "not" operator.
Good idea, fixed!

https://codereview.chromium.org/11054014/diff/1/chrome/test/pyautolib/remote_...
chrome/test/pyautolib/remote_inspector_client.py:736: def
GetNumberOfInstrumentedObjectsNotInHeap(self):
On 2012/10/04 01:16:33, dennis_jeffrey wrote:
> add a docstring and mention what is returned if the value isn't found.

Done.

https://codereview.chromium.org/11054014/diff/1/chrome/test/pyautolib/remote_...
chrome/test/pyautolib/remote_inspector_client.py:741: return -1
On 2012/10/04 01:16:33, dennis_jeffrey wrote:
> same comment as line 734 above

Done.

[dennis_jeffrey, 2012-10-04 22:02:57]

LGTM with an optional nit.

https://codereview.chromium.org/11054014/diff/6001/chrome/test/pyautolib/remo...
File chrome/test/pyautolib/remote_inspector_client.py (right):

https://codereview.chromium.org/11054014/diff/6001/chrome/test/pyautolib/remo...
chrome/test/pyautolib/remote_inspector_client.py:738: None if the number is not
available.
nit: I'd prefer the "Returns:" section to fully describe what is returned by
this function, rather than only stating what is returned in the exceptional
case.  But that would basically cause you to have to repeat what's in the
one-line header of this docstring, which adds redundancy.  So while it seems
weird to me that the "Returns:" section itself only tells part of the story, I
think it's probably ok here.

[yurys, 2012-10-05 07:34:52]

https://codereview.chromium.org/11054014/diff/6001/chrome/test/pyautolib/remo...
File chrome/test/pyautolib/remote_inspector_client.py (right):

https://codereview.chromium.org/11054014/diff/6001/chrome/test/pyautolib/remo...
chrome/test/pyautolib/remote_inspector_client.py:738: None if the number is not
available.
On 2012/10/04 22:02:57, dennis_jeffrey wrote:
> nit: I'd prefer the "Returns:" section to fully describe what is returned by
> this function, rather than only stating what is returned in the exceptional
> case.  But that would basically cause you to have to repeat what's in the
> one-line header of this docstring, which adds redundancy.  So while it seems
> weird to me that the "Returns:" section itself only tells part of the story, I
> think it's probably ok here.

Done. Changed the text to "Number of known instrumented objects or None if it is
not available."

Personally I don't like the fact that the information is repeated 4 times: 1) in
the method's name; 2) in the list of public methods; 3) in the method's 1-line
description; 4) in the return value description. The method itself is trivial
and it would be easier to skim through its code to learn details that are not
obvious from its name(like what it returns if the number is not available) than
reading all the commentaries.

[commit-bot: I haz the power, 2012-10-05 07:37:55]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/yurys@chromium.org/11054014/12001

[commit-bot: I haz the power, 2012-10-05 09:57:32]

Change committed as 160344