[GTK] Skip FontConfig initialization in WebKitTestRunner if requested
Created attachment 209267 [details] Patch
As described in the change log, FontConfig initialization is pretty heavy computation-wise, and is as such unnecessarily shown as a bottleneck when reviewing the profile. This skews the whole profile and obstructs real problems as the FontConfig setup is not really necessary when not running layout tests.
It is necessary to produce appropriate test results though, right?
When running layout tests, yes. This shouldn't apply to the performace tests (i.e. when running run-perf-tests and using a profiler), at least at the moment there are no tests testing the performance of anything that strictly relies on specific fonts being present. I'll think this through before pushing forward - in worst case, the FontConfig functions consuming substantial amount of time could be simply ignored when reviewing the profiles. Unfortunately the perf profiler doesn't provide any obvious mechanisms to ignore specific symbols at either record- or report-time.
Comment on attachment 209267 [details] Patch I'll spin off the typo fix into a separate patch.
(In reply to comment #4) > This shouldn't apply to the performace tests (i.e. when running run-perf-tests and using a profiler), at least at the moment there are no tests testing the performance of anything that strictly relies on specific fonts being present. I see. You make a good point. As long as it does not cause tests to start failing, I have no objection.
Created attachment 221110 [details] Patch
Comment on attachment 221110 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=221110&action=review > Tools/WebKitTestRunner/InjectedBundle/gtk/ActivateFontsGtk.cpp:87 > + if (g_getenv("WKTR_SKIP_FONTCONFIG_INITIALIZATION")) > + return; > + A "WEBKIT" prefix probably makes more sense here, since that's the one we traditionally use.
Comment on attachment 221110 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=221110&action=review >> Tools/WebKitTestRunner/InjectedBundle/gtk/ActivateFontsGtk.cpp:87 >> + > > A "WEBKIT" prefix probably makes more sense here, since that's the one we traditionally use. I'd then change the variable name to 'WEBKIT_SKIP_WKTR_FONTCONFIG_INITIALIZATION'. Sounds OK?
(In reply to comment #9) > (From update of attachment 221110 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=221110&action=review > > >> Tools/WebKitTestRunner/InjectedBundle/gtk/ActivateFontsGtk.cpp:87 > >> + > > > > A "WEBKIT" prefix probably makes more sense here, since that's the one we traditionally use. > > I'd then change the variable name to 'WEBKIT_SKIP_WKTR_FONTCONFIG_INITIALIZATION'. Sounds OK? I'm not a big fan unnecessary abbreviation, so I prefer WEBKIT_SKIP_WEBKITTESTRUNNER_FONTCONFIG_INITIALIZATION.
Committed r161990: <http://trac.webkit.org/changeset/161990>