Bug 114931 – Remove the memory instrumentation code

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector

[Benjamin Poulain]

That huge dead code is useless.

[Radar WebKit Bug Importer]

<rdar://problem/13703130>

[Benjamin Poulain]

Created attachment 198983 [details]
Patch

[EFL EWS Bot]

Comment on attachment 198983 [details]
Patch

Attachment 198983 [details] did not pass efl-ews (efl):
Output: http://webkit-queues.appspot.com/results/199016

[kov's GTK+ EWS bot]

Comment on attachment 198983 [details]
Patch

Attachment 198983 [details] did not pass gtk-ews (gtk):
Output: http://webkit-queues.appspot.com/results/55431

[Benjamin Poulain]

Created attachment 198985 [details]
Patch

[EFL EWS Bot]

Comment on attachment 198985 [details]
Patch

Attachment 198985 [details] did not pass efl-ews (efl):
Output: http://webkit-queues.appspot.com/results/181140

[Benjamin Poulain]

Created attachment 198986 [details]
Patch

[Benjamin Poulain]

Created attachment 198987 [details]
Patch

[Benjamin Poulain]

Created attachment 198989 [details]
Patch

[kov's GTK+ EWS bot]

Comment on attachment 198989 [details]
Patch

Attachment 198989 [details] did not pass gtk-ews (gtk):
Output: http://webkit-queues.appspot.com/results/81558

[Early Warning System Bot]

Comment on attachment 198989 [details]
Patch

Attachment 198989 [details] did not pass qt-ews (qt):
Output: http://webkit-queues.appspot.com/results/200010

[Early Warning System Bot]

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

[Benjamin Poulain]

Created attachment 198990 [details]
Patch

[Benjamin Poulain]

Created attachment 198991 [details]
Patch

[Benjamin Poulain]

Comment on attachment 198991 [details]
Patch

Looks like two sick bots. It builds everywhere else -> moving to review?

[Andreas Kling]

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!

[Timothy Hatcher]

(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!

[Alexey Proskuryakov]

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.

[Joseph Pecoraro]

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.

[Timothy Hatcher]

There are a handful of protocol events and functions we can remove. I filed that as bug 114118.

[Benjamin Poulain]

Committed r148921: <http://trac.webkit.org/changeset/148921>

[Eric Seidel (no email)]

I'm curious if the size-win you saw was in Debug or Release?

[Benjamin Poulain]

(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 :)

[Eric Seidel (no email)]

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.

[Nico Weber]

(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.

[Csaba Osztrogonác]

(In reply to comment #21)
> Committed r148921: <http://trac.webkit.org/changeset/148921>

FYI: It broke the build on the Apple Windows bots.