120110 2013-08-21 08:21:15 -0700 [GTK] Skip FontConfig initialization in WebKitTestRunner if requested 2014-01-14 11:46:00 -0800 1 1 1 Unclassified WebKit New Bugs 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 zan zan mrobinson oldest_to_newest 919415 0 zan 2013-08-21 08:21:15 -0700 [GTK] Skip FontConfig initialization in WebKitTestRunner if requested 919416 1 209267 zan 2013-08-21 08:23:31 -0700 Created attachment 209267 Patch 919418 2 zan 2013-08-21 08:26:15 -0700 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. 919419 3 mrobinson 2013-08-21 08:28:55 -0700 It is necessary to produce appropriate test results though, right? 919455 4 zan 2013-08-21 09:17:31 -0700 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. 919457 5 209267 zan 2013-08-21 09:19:11 -0700 Comment on attachment 209267 Patch I'll spin off the typo fix into a separate patch. 919465 6 mrobinson 2013-08-21 09:29:00 -0700 (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. 967686 7 221110 zan 2014-01-13 23:59:16 -0800 Created attachment 221110 Patch 967837 8 221110 mrobinson 2014-01-14 08:05:54 -0800 Comment on attachment 221110 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. 967866 9 221110 zan 2014-01-14 09:34:36 -0800 Comment on attachment 221110 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? 967939 10 mrobinson 2014-01-14 11:19:12 -0800 (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. 967958 11 zan 2014-01-14 11:46:00 -0800 Committed r161990: <http://trac.webkit.org/changeset/161990> 209267 2013-08-21 08:23:31 -0700 2014-01-13 23:59:10 -0800 Patch bug-120110-20130821082330.patch text/plain 1962 zan U3VidmVyc2lvbiBSZXZpc2lvbjogMTU0MzgwCmRpZmYgLS1naXQgYS9Ub29scy9DaGFuZ2VMb2cg Yi9Ub29scy9DaGFuZ2VMb2cKaW5kZXggMjM5NjJiOTMwMTBjNmI5NTIzMDZjNjkwZWY5OTJmZTZj ODNiMDczMC4uODEwYjViZTRmNjY5ODkwZTk5ZWFjMjgwNThkNGY1NzM4MDhjYTRhNCAxMDA2NDQK LS0tIGEvVG9vbHMvQ2hhbmdlTG9nCisrKyBiL1Rvb2xzL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE4 IEBACisyMDEzLTA4LTIxICBaYW4gRG9iZXJzZWsgIDx6ZG9iZXJzZWtAaWdhbGlhLmNvbT4KKwor ICAgICAgICA8aHR0cHM6Ly93ZWJraXQub3JnL2IvMTIwMTEwPiBbR1RLXSBTa2lwIEZvbnRDb25m aWcgaW5pdGlhbGl6YXRpb24gaW4gV2ViS2l0VGVzdFJ1bm5lciBpZiByZXF1ZXN0ZWQKKworICAg ICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBTa2lwIHRoZSBGb250 Q29uZmlnIHNldHRpbmdzIGluaXRpYWxpemF0aW9uIGluIFdlYktpdFRlc3RSdW5uZXIgaWYgdGhl CisgICAgICAgIFdLVFJfU0tJUF9GT05UQ09ORklHX0lOSVRJQUxJWkFUSU9OIGVudmlyb25tZW50 IHZhcmlhYmxlIGlzIHNldC4gVGhpcworICAgICAgICBpcyB1c2VmdWwgd2hlbiBwcm9maWxpbmcg dGhlIFdlYktpdFRlc3RSdW5uZXIgZXhlY3V0aW9uLCB3aGVyZSBhdCB0aGUgc3RhcnR1cAorICAg ICAgICB0aGlzIHN0ZXAgdGFrZXMgY29uc2lkZXJhYmxlIGFtb3VudCBvZiB0aW1lIGFuZCBkaXN0 dXJicyB0aGUgcHJvZmlsaW5nIG91dHB1dC4KKworICAgICAgICAqIFdlYktpdFRlc3RSdW5uZXIv SW5qZWN0ZWRCdW5kbGUvZ3RrL0FjdGl2YXRlRm9udHNHdGsuY3BwOgorICAgICAgICAoV1RSOjpp bml0aWFsaXplRm9udENvbmZpZ1NldHRpbmcpOiBGaXggYSB0eXBvIC0gaW5pdGl0aWFsaXplIC0+ IGluaXRpYWxpemUuCisgICAgICAgIChXVFI6OmFjdGl2YXRlRm9udHMpOgorCiAyMDEzLTA4LTIw ICBBbGV4IENocmlzdGVuc2VuICA8YWNocmlzdGVuc2VuQGFwcGxlLmNvbT4KIAogICAgICAgICBb V2luZG93c10gTGlua2luZyBmaXggZm9yIFdpbjY0LgpkaWZmIC0tZ2l0IGEvVG9vbHMvV2ViS2l0 VGVzdFJ1bm5lci9JbmplY3RlZEJ1bmRsZS9ndGsvQWN0aXZhdGVGb250c0d0ay5jcHAgYi9Ub29s cy9XZWJLaXRUZXN0UnVubmVyL0luamVjdGVkQnVuZGxlL2d0ay9BY3RpdmF0ZUZvbnRzR3RrLmNw cAppbmRleCA2ZTUxMDM1MjI5YmQxNzAxYzg0NTNjYzE4YzJlNDEzZWJmZWI1MDNjLi5jMmZlYWI4 MzA5MzIzYTc4ZDExYWNiMjBkNzlkOGMwN2RjODNiNzM3IDEwMDY0NAotLS0gYS9Ub29scy9XZWJL aXRUZXN0UnVubmVyL0luamVjdGVkQnVuZGxlL2d0ay9BY3RpdmF0ZUZvbnRzR3RrLmNwcAorKysg Yi9Ub29scy9XZWJLaXRUZXN0UnVubmVyL0luamVjdGVkQnVuZGxlL2d0ay9BY3RpdmF0ZUZvbnRz R3RrLmNwcApAQCAtODAsOCArODAsMTEgQEAgc3RhdGljIENTdHJpbmcgZ2V0Rm9udHNQYXRoKCkK ICAgICByZXR1cm4gQ1N0cmluZygpOwogfQogCi12b2lkIGluaXRpdGlhbGl6ZUZvbnRDb25maWdT ZXR0aW5nKCkKK3ZvaWQgaW5pdGlhbGl6ZUZvbnRDb25maWdTZXR0aW5nKCkKIHsKKyAgICBpZiAo Z19nZXRlbnYoIldLVFJfU0tJUF9GT05UQ09ORklHX0lOSVRJQUxJWkFUSU9OIikpCisgICAgICAg IHJldHVybjsKKwogICAgIEZjSW5pdCgpOwogCiAgICAgLy8gSWYgYSB0ZXN0IHJlc3VsdGVkIGEg Zm9udCBiZWluZyBhZGRlZCBvciByZW1vdmVkIHZpYSB0aGUgQGZvbnQtZmFjZSBydWxlLCB0aGVu CkBAIC0xNTEsNyArMTU0LDcgQEAgdm9pZCBpbml0aXRpYWxpemVGb250Q29uZmlnU2V0dGluZygp CiB2b2lkIGFjdGl2YXRlRm9udHMoKQogewogICAgIGluaXRpYWxpemVHdGtTZXR0aW5ncygpOwot ICAgIGluaXRpdGlhbGl6ZUZvbnRDb25maWdTZXR0aW5nKCk7CisgICAgaW5pdGlhbGl6ZUZvbnRD b25maWdTZXR0aW5nKCk7CiB9CiAKIH0K 221110 2014-01-13 23:59:16 -0800 2014-01-14 09:34:36 -0800 Patch bug-120110-20140114085914.patch text/plain 1548 zan SW5kZXg6IFRvb2xzL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBUb29scy9DaGFuZ2VMb2cJKHJl dmlzaW9uIDE2MTk1MikKKysrIFRvb2xzL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwz ICsxLDE4IEBACisyMDE0LTAxLTEzICBaYW4gRG9iZXJzZWsgIDx6ZG9iZXJzZWtAaWdhbGlhLmNv bT4KKworICAgICAgICBbR1RLXSBTa2lwIEZvbnRDb25maWcgaW5pdGlhbGl6YXRpb24gaW4gV2Vi S2l0VGVzdFJ1bm5lciBpZiByZXF1ZXN0ZWQKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5v cmcvc2hvd19idWcuY2dpP2lkPTEyMDExMAorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAo T09QUyEpLgorCisgICAgICAgIFNraXAgdGhlIEZvbnRDb25maWcgc2V0dGluZ3MgaW5pdGlhbGl6 YXRpb24gaW4gV2ViS2l0VGVzdFJ1bm5lciBpZiB0aGUKKyAgICAgICAgV0tUUl9TS0lQX0ZPTlRD T05GSUdfSU5JVElBTElaQVRJT04gZW52aXJvbm1lbnQgdmFyaWFibGUgaXMgc2V0LiBUaGlzCisg ICAgICAgIGlzIHVzZWZ1bCB3aGVuIHByb2ZpbGluZyB0aGUgV2ViS2l0VGVzdFJ1bm5lciBleGVj dXRpb24sIHdoZXJlIGF0IHRoZSBzdGFydHVwCisgICAgICAgIHRoaXMgc3RlcCB0YWtlcyBjb25z aWRlcmFibGUgYW1vdW50IG9mIHRpbWUgYW5kIGRpc3R1cmJzIHRoZSBwcm9maWxpbmcgb3V0cHV0 LgorCisgICAgICAgICogV2ViS2l0VGVzdFJ1bm5lci9JbmplY3RlZEJ1bmRsZS9ndGsvQWN0aXZh dGVGb250c0d0ay5jcHA6CisgICAgICAgIChXVFI6OmluaXRpYWxpemVGb250Q29uZmlnU2V0dGlu Zyk6CisKIDIwMTQtMDEtMTMgIEx1Y2FzIEZvcnNjaGxlciAgPGxmb3JzY2hsZXJAYXBwbGUuY29t PgogCiAgICAgICAgIFVwZGF0ZSB0aGUgTExJTlQgQ0xvb3AgQm90IGJ1aWxkIGluZnJhc3RydWN0 dXJlCkluZGV4OiBUb29scy9XZWJLaXRUZXN0UnVubmVyL0luamVjdGVkQnVuZGxlL2d0ay9BY3Rp dmF0ZUZvbnRzR3RrLmNwcAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBUb29scy9XZWJLaXRUZXN0UnVubmVyL0lu amVjdGVkQnVuZGxlL2d0ay9BY3RpdmF0ZUZvbnRzR3RrLmNwcAkocmV2aXNpb24gMTYxOTUxKQor KysgVG9vbHMvV2ViS2l0VGVzdFJ1bm5lci9JbmplY3RlZEJ1bmRsZS9ndGsvQWN0aXZhdGVGb250 c0d0ay5jcHAJKHdvcmtpbmcgY29weSkKQEAgLTgyLDYgKzgyLDkgQEAgc3RhdGljIENTdHJpbmcg Z2V0Rm9udHNQYXRoKCkKIAogdm9pZCBpbml0aWFsaXplRm9udENvbmZpZ1NldHRpbmcoKQogewor ICAgIGlmIChnX2dldGVudigiV0tUUl9TS0lQX0ZPTlRDT05GSUdfSU5JVElBTElaQVRJT04iKSkK KyAgICAgICAgcmV0dXJuOworCiAgICAgRmNJbml0KCk7CiAKICAgICAvLyBJZiBhIHRlc3QgcmVz dWx0ZWQgYSBmb250IGJlaW5nIGFkZGVkIG9yIHJlbW92ZWQgdmlhIHRoZSBAZm9udC1mYWNlIHJ1 bGUsIHRoZW4K