WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
91274
Add FastMalloc statistics in window.internals
https://bugs.webkit.org/show_bug.cgi?id=91274
Summary
Add FastMalloc statistics in window.internals
Olivier Blin
Reported
2012-07-13 13:28:29 PDT
FastMalloc statistics (reservedFastMalloc, committedFastMalloc, freeListFastMalloc) should be exposed in window.internals, to be used for
bug 78984
and
bug 90858
Also see
bug 91204
for more background.
Attachments
Patch
(2.93 KB, patch)
2012-07-18 08:18 PDT
,
Olivier Blin
no flags
Details
Formatted Diff
Diff
Patch
(9.78 KB, patch)
2012-07-19 09:16 PDT
,
Olivier Blin
no flags
Details
Formatted Diff
Diff
Patch
(15.15 KB, patch)
2012-07-19 11:00 PDT
,
Olivier Blin
no flags
Details
Formatted Diff
Diff
Patch
(18.23 KB, patch)
2012-07-19 14:32 PDT
,
Olivier Blin
no flags
Details
Formatted Diff
Diff
Patch
(27.60 KB, patch)
2012-07-20 02:54 PDT
,
Olivier Blin
no flags
Details
Formatted Diff
Diff
Patch
(25.86 KB, patch)
2012-07-20 03:59 PDT
,
Olivier Blin
no flags
Details
Formatted Diff
Diff
Patch
(25.86 KB, patch)
2012-07-26 06:13 PDT
,
Olivier Blin
no flags
Details
Formatted Diff
Diff
Show Obsolete
(6)
View All
Add attachment
proposed patch, testcase, etc.
Zoltan Horvath
Comment 1
2012-07-18 01:56:11 PDT
What is the progress of the patch? Are you working on it?
Olivier Blin
Comment 2
2012-07-18 02:30:39 PDT
I could use some advice about the interfaces. Should we duplicate (move?) the MemoryInfo object in window.internals? Or do we just add new interfaces for the malloc stats directly in window.internals? unsigned long reservedVMBytes(); unsigned long committedVMBytes(); unsigned long freeListVMBytes();
Zoltan Horvath
Comment 3
2012-07-18 02:40:20 PDT
(In reply to
comment #2
)
> I could use some advice about the interfaces. > Should we duplicate (move?) the MemoryInfo object in window.internals?
We shouldn't duplicate.
> Or do we just add new interfaces for the malloc stats directly in window.internals? > unsigned long reservedVMBytes(); > unsigned long committedVMBytes(); > unsigned long freeListVMBytes();
I think this would be just fine.
Zoltan Horvath
Comment 4
2012-07-18 03:26:30 PDT
(In reply to
comment #2
)
> unsigned long reservedVMBytes(); > unsigned long committedVMBytes(); > unsigned long freeListVMBytes();
Reserved and committed will be enough for us.
Olivier Blin
Comment 5
2012-07-18 08:18:05 PDT
Created
attachment 153017
[details]
Patch
Olivier Blin
Comment 6
2012-07-18 08:18:30 PDT
Is there any test I can add?
Ryosuke Niwa
Comment 7
2012-07-18 14:15:43 PDT
Comment on
attachment 153017
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=153017&action=review
> Source/WebCore/testing/Internals.idl:181 > + unsigned long reservedVMBytes(); > + unsigned long committedVMBytes(); > + unsigned long freeListBytes();
Would have been better if you've added a property like fastMallocStatistics and added these methods there. Also, we should make these readonly properties instead of methods.
Olivier Blin
Comment 8
2012-07-18 14:58:25 PDT
I just followed Zoltan's advice in
comment #3
, but ok, I can move that into a new Source/WebCore/testing/FastMallocStatistics.idl with read-only attributes. What kind of tests can I add? Would it be ok to just check for definedness and having attributes >= 0?
Ryosuke Niwa
Comment 9
2012-07-18 15:05:09 PDT
(In reply to
comment #8
)
> I just followed Zoltan's advice in
comment #3
, but ok, I can move that into a new Source/WebCore/testing/FastMallocStatistics.idl with read-only attributes. > > What kind of tests can I add? > Would it be ok to just check for definedness and having attributes >= 0?
Yes. Maybe you can add it to LayoutTests/fast/harness?
Olivier Blin
Comment 10
2012-07-19 09:16:38 PDT
Created
attachment 153276
[details]
Patch
Olivier Blin
Comment 11
2012-07-19 09:17:29 PDT
This patch is still missing tests and more build system integration. Zoltan, Ryosuke: am I going in the good direction for your needs?
WebKit Review Bot
Comment 12
2012-07-19 09:27:12 PDT
Comment on
attachment 153276
[details]
Patch
Attachment 153276
[details]
did not pass chromium-ews (chromium-xvfb): Output:
http://queues.webkit.org/results/13269957
Build Bot
Comment 13
2012-07-19 09:40:13 PDT
Comment on
attachment 153276
[details]
Patch
Attachment 153276
[details]
did not pass win-ews (win): Output:
http://queues.webkit.org/results/13278925
Early Warning System Bot
Comment 14
2012-07-19 10:41:57 PDT
Comment on
attachment 153276
[details]
Patch
Attachment 153276
[details]
did not pass qt-wk2-ews (qt): Output:
http://queues.webkit.org/results/13279998
Olivier Blin
Comment 15
2012-07-19 11:00:28 PDT
Created
attachment 153308
[details]
Patch
Build Bot
Comment 16
2012-07-19 11:36:43 PDT
Comment on
attachment 153308
[details]
Patch
Attachment 153308
[details]
did not pass win-ews (win): Output:
http://queues.webkit.org/results/13272910
Build Bot
Comment 17
2012-07-19 12:21:53 PDT
Comment on
attachment 153308
[details]
Patch
Attachment 153308
[details]
did not pass mac-ews (mac): Output:
http://queues.webkit.org/results/13307009
Gyuyoung Kim
Comment 18
2012-07-19 13:04:56 PDT
Comment on
attachment 153308
[details]
Patch
Attachment 153308
[details]
did not pass efl-ews (efl): Output:
http://queues.webkit.org/results/13301066
Olivier Blin
Comment 19
2012-07-19 14:32:22 PDT
Created
attachment 153346
[details]
Patch
Build Bot
Comment 20
2012-07-19 14:40:24 PDT
Comment on
attachment 153346
[details]
Patch
Attachment 153346
[details]
did not pass mac-ews (mac): Output:
http://queues.webkit.org/results/13305059
Olivier Blin
Comment 21
2012-07-20 02:54:32 PDT
Created
attachment 153459
[details]
Patch
Olivier Blin
Comment 22
2012-07-20 03:59:25 PDT
Created
attachment 153475
[details]
Patch remove double changelog entries
Ryosuke Niwa
Comment 23
2012-07-20 11:09:14 PDT
Comment on
attachment 153475
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=153475&action=review
> Source/WebCore/testing/FastMallocStatistics.h:46 > + m_stats = WTF::fastMallocStatistics();
If we're storing the statistics at the object creation, then we probably need to make the creation explicit instead of making it hang off of internals.
> Source/WebCore/testing/Internals.idl:179 > + readonly attribute FastMallocStatistics fastMallocStatistics;
i.e. this should be a function call instead of being an object in order to allow getting statistics multiple times without being dependent on GC behavior.
> LayoutTests/fast/harness/fastmallocstatistics-object.html:1 > +<html>
Missing DOCTYPE.
Olivier Blin
Comment 24
2012-07-23 08:12:01 PDT
(In reply to
comment #23
)
> (From update of
attachment 153475
[details]
) > View in context:
https://bugs.webkit.org/attachment.cgi?id=153475&action=review
> > > Source/WebCore/testing/FastMallocStatistics.h:46 > > + m_stats = WTF::fastMallocStatistics(); > > If we're storing the statistics at the object creation, then we probably need to make the creation explicit instead of making it hang off of internals.
>
> > Source/WebCore/testing/Internals.idl:179 > > + readonly attribute FastMallocStatistics fastMallocStatistics; > > i.e. this should be a function call instead of being an object in order to allow getting statistics multiple times without being dependent on GC behavior.
Thanks, I will change it to a function call. I followed the model used for console.memory, which uses a readonly attribute as well. Does it mean that getting fresh stats in console.memory would depend on the GC?
> > LayoutTests/fast/harness/fastmallocstatistics-object.html:1 > > +<html> > > Missing DOCTYPE.
Ok, I will add one (some tests in the harness dir are missing it as well). Thanks for the review
Olivier Blin
Comment 25
2012-07-26 06:13:59 PDT
Created
attachment 154632
[details]
Patch
Olivier Blin
Comment 26
2012-07-26 06:17:00 PDT
This update converts the readonly attribute in window.internals to a function, and adds a DOCTYPE to the test. The Xcode project has been manually edited to add the new files (with manually generated GUIDs), but it seems it eventually passed the MaC bot.
Ryosuke Niwa
Comment 27
2012-07-26 10:34:23 PDT
Comment on
attachment 154632
[details]
Patch Yay! Looks great to me.
WebKit Review Bot
Comment 28
2012-07-26 11:17:17 PDT
Comment on
attachment 154632
[details]
Patch Clearing flags on attachment: 154632 Committed
r123773
: <
http://trac.webkit.org/changeset/123773
>
WebKit Review Bot
Comment 29
2012-07-26 11:17:26 PDT
All reviewed patches have been landed. Closing bug.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug