Created attachment 40554 [details] Timing test, WebkitWebView vs. GtkTextView I need to put a fair number of WebkitWebViews into one GTK application, and it takes over a second for my window to be built. (I am using Webkit-GTK 1.1.10.) The constructor webkit_web_view_new() is very slow. Please see attached program for a timing test. I have compared creating twenty WebkitWebViews to creating twenty GtkTextViews, which is also a fairly complicated widget. Compile with: gcc -o testbug testbug.c `pkg-config --cflags --libs gtk+-2.0 webkit-1.0` Typical output: webkit_web_view_new() time: 0.036524 webkit_web_view_new() time: 0.041833 webkit_web_view_new() time: 0.029860 webkit_web_view_new() time: 0.028802 webkit_web_view_new() time: 0.028518 webkit_web_view_new() time: 0.028611 webkit_web_view_new() time: 0.028586 webkit_web_view_new() time: 0.028596 webkit_web_view_new() time: 0.029057 webkit_web_view_new() time: 0.028576 webkit_web_view_new() time: 0.028491 webkit_web_view_new() time: 0.028798 webkit_web_view_new() time: 0.028640 webkit_web_view_new() time: 0.028650 webkit_web_view_new() time: 0.031685 webkit_web_view_new() time: 0.028799 webkit_web_view_new() time: 0.028759 webkit_web_view_new() time: 0.028816 webkit_web_view_new() time: 0.029005 webkit_web_view_new() time: 0.029732 gtk_text_view_new() time: 0.000489 gtk_text_view_new() time: 0.000018 gtk_text_view_new() time: 0.000023 gtk_text_view_new() time: 0.000017 gtk_text_view_new() time: 0.000016 gtk_text_view_new() time: 0.000016 gtk_text_view_new() time: 0.000020 gtk_text_view_new() time: 0.000016 gtk_text_view_new() time: 0.000016 gtk_text_view_new() time: 0.000019 gtk_text_view_new() time: 0.000016 gtk_text_view_new() time: 0.000015 gtk_text_view_new() time: 0.000020 gtk_text_view_new() time: 0.000018 gtk_text_view_new() time: 0.000016 gtk_text_view_new() time: 0.000020 gtk_text_view_new() time: 0.000019 gtk_text_view_new() time: 0.000017 gtk_text_view_new() time: 0.000025 gtk_text_view_new() time: 0.000016 Total time: 0.714716
Well, first of all GtkTextView is nowhere near as complex as WebKitWebView, since a webview comes with many hundreds of thousands of C++ lines behind. That being said, just profile the creation of the widget with sysprof or similar and we'll see what's taking the time.
Created attachment 40660 [details] Profiler output of timing test from sysprof
Thanks for the tip, I didn't know about sysprof. It is nifty. I hope I understood how to use it and interpret the output correctly. The output is attached, look for the program [testbug]. It seems that the bottleneck is enchant_broker_request_dict(), called in webkit_web_settings_set_property() in webkitwebsettings.cpp. The default value for the "spell-checking-languages" property is NULL, meaning load the dictionary for GTK's default language, so at least one dictionary gets loaded whenever you create a WebView. Would it work to avoid loading the dictionary if the "enable-spell-checking" property is FALSE (as it is by default)?
Created attachment 43653 [details] Make EnchantBroker a singleton According to http://sourceforge.net/tracker/?func=detail&aid=2642198&group_id=7896&atid=107896 EnchantBroker should be a singleton. This patch: - solves the issue, obviously - simplifies things (removal of SpellLanguage struct) - renames some variables to better reflect what they are - gives a more meaningful name to get_spell_languages method (get_enchant_dicts) - adds a missing g_free() - removes "spell-checking-languages-list" from the _copy() thing, because it's not even a property (?) - applies to trunk too - is written by an annoyed user, so please bear with me :)
Created attachment 47009 [details] Make EnchantBroker a singleton (updated) This is an updated version of the patch above, so it applies without fuzz against WebKitGTK 1.1.19. No other changes were made. How to see a really big slowdown caused by this bug: 1. get a copy of Polish myspell dictionary from http://www.sjp.pl/slownik/en/ 2. extract pl_PL.{aff,dic} to ~/.config/enchant/myspell/ 3. restart your favorite browser 4. cry
I agree Krzysztof, enchant does a crazy job during startup. You can feel with with multiply dictionaties installed. So me++ for the patch :) The problem is does patch really needs to break existing api? I think it can be done without api changes.
ping? Patch still applies fine to current 1.1.20. Can it be reviewed?
(In reply to comment #7) > ping? > > Patch still applies fine to current 1.1.20. Can it be reviewed? You need to set the Review flag to "?" in patch details so it pops up on list of reviewable patches (unless you harass some developer on IRC or mailinglist to do it..) :S
Comment on attachment 47009 [details] Make EnchantBroker a singleton (updated) Every patch requires a ChangeLog. This patch would not apply to our repository as is anyway due ot the "webkit-1.1.9" path prefixes. See http://webkit.org/coding/contributing.html
Created attachment 49131 [details] Updated patch (w/ ChangeLog & style issues fixed) I think this is a very good change, so I've taken the liberty of cleaning the patch up a bit and attaching a ChangeLog.
Attachment 49131 [details] did not build on gtk: Build output: http://webkit-commit-queue.appspot.com/results/290665
Attachment 49131 [details] did not pass style-queue: Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1 WebKit/gtk/webkit/webkitwebsettings.cpp:833: An else statement can be removed when the prior "if" concludes with a return, break, continue or goto statement. [readability/control_flow] [4] Total errors found: 1 in 4 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 49131 [details] Updated patch (w/ ChangeLog & style issues fixed) 1276 GSList* webkit_web_settings_get_spell_languages(WebKitWebView *web_view) 1272 GSList* webkit_web_settings_get_enchant_dicts(WebKitWebView* webView) 12771273 { 12781274 g_return_val_if_fail(WEBKIT_IS_WEB_VIEW(web_view), 0); 12791275 12801276 WebKitWebSettings* settings = webkit_web_view_get_settings(web_view); Looks like you need to adjust the variable name in the two calls above =). r+ with that change, and removing the else clause, as pointed by the style bot, if you feel like committing it, Martin!
Committed r55222: <http://trac.webkit.org/changeset/55222>