| Summary: | Do not do any logging initialization when logging is disabled | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Benjamin Poulain <benjamin> | ||||||||||||
| Component: | WebKit Misc. | Assignee: | Benjamin Poulain <benjamin> | ||||||||||||
| Status: | RESOLVED FIXED | ||||||||||||||
| Severity: | Normal | CC: | cmarcelo, danw, ddkilzer, eric.carlson, eric, feature-media-reviews, gustavo, japhet, jochen, joepeck, menard, mifenton, mrobinson, pnormand, rakuco, webkit.review.bot, xan.lopez, zoltan | ||||||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||||||
| Version: | 528+ (Nightly build) | ||||||||||||||
| Hardware: | Unspecified | ||||||||||||||
| OS: | Unspecified | ||||||||||||||
| Attachments: |
|
||||||||||||||
|
Description
Benjamin Poulain
2012-06-28 17:00:40 PDT
Created attachment 150044 [details]
Patch
Comment on attachment 150044 [details] Patch Attachment 150044 [details] did not pass qt-wk2-ews (qt): Output: http://queues.webkit.org/results/13108240 Comment on attachment 150044 [details] Patch Attachment 150044 [details] did not pass gtk-ews (gtk): Output: http://queues.webkit.org/results/13119163 Comment on attachment 150044 [details] Patch Attachment 150044 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/13114181 Created attachment 150055 [details]
Patch
Comment on attachment 150055 [details] Patch Attachment 150055 [details] did not pass gtk-ews (gtk): Output: http://queues.webkit.org/results/13121124 Created attachment 150059 [details]
Patch
Looks like GTK has a strange use of WebCore logging :( Comment on attachment 150059 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=150059&action=review > Source/WebCore/ChangeLog:8 > + Initializating of the logging channels was taking time on startup. When loggin is disabled "loggin" > Source/WebCore/ChangeLog:9 > + (and the LOG macro do nothing), we should aslo disable logging channels and initialization. do nothing -> does nothing. aslo. > Source/WebCore/ChangeLog:12 > + The biggest part of the patch is the change from !LOG_DISABLED to using ENABLE(LOGGING). > + This is done for consistency with the other ENABLE features used in WebKit. But I don't think logging is like other ENABLE features. ENABLE is used for web- or user- facing features, and doesn't normally change between release and debug builds. LOGGING is more like ASSERTS, in that it's disabled in release. Comment on attachment 150059 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=150059&action=review > Source/WebCore/ChangeLog:9 > + Initializating of the logging channels was taking time on startup. When loggin is disabled > + (and the LOG macro do nothing), we should aslo disable logging channels and initialization. Nit (typo): "Initializating" => "Initializing" Nit (typo): "loggin" => "logging" Nit (typo): "macro do nothing" => "macro does nothing" Nit (typo): "aslo" => "also" Created attachment 150263 [details]
Patch
Comment on attachment 150263 [details]
Patch
I'll look into using Assertions.h directly for WebCore.exp.in.
Comment on attachment 150263 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=150263&action=review > Source/WebCore/WebCore.exp.in:631 > +#if !LOG_DISABLED > __ZN7WebCore36initializeLoggingChannelsIfNecessaryEv > +#endif // !LOG_DISABLED The convention seems to be adding a new block near the bottom of the file. #if !defined(LOG_DISABLED) .... #endif I assume other build systems (like Windows?) might need a similar export fix. This patch is much easier to follow =) Created attachment 150297 [details]
Patch
Comment on attachment 150297 [details]
Patch
Looks find on EFL port case.
Committed r121707: <http://trac.webkit.org/changeset/121707> |