| Summary: | Remove the memory instrumentation code | ||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Benjamin Poulain <benjamin> | ||||||||||||||||
| Component: | Web Inspector | Assignee: | Benjamin Poulain <benjamin> | ||||||||||||||||
| Status: | RESOLVED FIXED | ||||||||||||||||||
| Severity: | Normal | CC: | eflews.bot, eric, graouts, gtk-ews, gyuyoung.kim, joepeck, ossy, philn, rego+ews, thakis, timothy, webkit-bug-importer, webkit-ews, xan.lopez | ||||||||||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||||||||||
| Version: | 528+ (Nightly build) | ||||||||||||||||||
| Hardware: | Unspecified | ||||||||||||||||||
| OS: | Unspecified | ||||||||||||||||||
| Attachments: |
|
||||||||||||||||||
|
Description
Benjamin Poulain
2013-04-21 19:54:03 PDT
Created attachment 198983 [details]
Patch
Comment on attachment 198983 [details] Patch Attachment 198983 [details] did not pass efl-ews (efl): Output: http://webkit-queues.appspot.com/results/199016 Comment on attachment 198983 [details] Patch Attachment 198983 [details] did not pass gtk-ews (gtk): Output: http://webkit-queues.appspot.com/results/55431 Created attachment 198985 [details]
Patch
Comment on attachment 198985 [details] Patch Attachment 198985 [details] did not pass efl-ews (efl): Output: http://webkit-queues.appspot.com/results/181140 Created attachment 198986 [details]
Patch
Created attachment 198987 [details]
Patch
Created attachment 198989 [details]
Patch
Comment on attachment 198989 [details] Patch Attachment 198989 [details] did not pass gtk-ews (gtk): Output: http://webkit-queues.appspot.com/results/81558 Comment on attachment 198989 [details] Patch Attachment 198989 [details] did not pass qt-ews (qt): Output: http://webkit-queues.appspot.com/results/200010 Comment on attachment 198989 [details] Patch Attachment 198989 [details] did not pass qt-wk2-ews (qt-wk2): Output: http://webkit-queues.appspot.com/results/80224 Created attachment 198990 [details]
Patch
Created attachment 198991 [details]
Patch
Comment on attachment 198991 [details]
Patch
Looks like two sick bots. It builds everywhere else -> moving to review?
Comment on attachment 198991 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=198991&action=review This certainly seems like the right thing to do. r=me. > Source/WebCore/ChangeLog:15 > + By removing the code, the binary become 1240976 bytes smaller. > + Yep, almost 1 Mb, bringing WebCore to the size it has 5 months ago :) Holy halloumi, Batman! (In reply to comment #16) > (From update of attachment 198991 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=198991&action=review > > This certainly seems like the right thing to do. r=me. > > > Source/WebCore/ChangeLog:15 > > + By removing the code, the binary become 1240976 bytes smaller. > > + Yep, almost 1 Mb, bringing WebCore to the size it has 5 months ago :) > > Holy halloumi, Batman! Nice! Comment on attachment 198991 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=198991&action=review > Source/WebCore/ChangeLog:24 > + * CMakeLists.txt: > + * GNUmakefile.list.am: > + * Modules/webaudio/AudioContext.cpp: > + * Modules/webaudio/AudioContext.h: > + (AudioContext): > + * Modules/webaudio/AudioNode.cpp: > + * Modules/webaudio/AudioNode.h: > + (AudioNode): Can you please edit this list to make it shorter/nicer, or even remove it altogether? I can't imagine any use for it in the ChangeLog. Comment on attachment 198991 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=198991&action=review Nice size saving. > Source/WebCore/inspector/Inspector.json:79 > "events": [ Unless someone can think of a good reason to leave it in, this should also remove the "addNativeSnapshotChunk" event in the memory domain. The event can no longer be generated by our backend. There are a handful of protocol events and functions we can remove. I filed that as bug 114118. Committed r148921: <http://trac.webkit.org/changeset/148921> I'm curious if the size-win you saw was in Debug or Release? (In reply to comment #22) > I'm curious if the size-win you saw was in Debug or Release? Release. My laptop is a little slow for debug builds :) I've filed a bug in Chromium about tracking the effect of this on our binary size: https://code.google.com/p/chromium/issues/detail?id=234422 Interesting that Nico's data showed this as only 300kb for Chroimum. I wonder if we're compiling with some clang/gcc flag which WebKit should consider. (In reply to comment #24) > I've filed a bug in Chromium about tracking the effect of this on our binary size: > https://code.google.com/p/chromium/issues/detail?id=234422 > > Interesting that Nico's data showed this as only 300kb for Chroimum. I wonder if we're compiling with some clang/gcc flag which WebKit should consider. Might just be stripped vs not stripped. (In reply to comment #21) > Committed r148921: <http://trac.webkit.org/changeset/148921> FYI: It broke the build on the Apple Windows bots. |