Bug 30032 – [Gtk] Creation of a WebkitWebView widget is very slow

• Component: WebKitGTK
• Status: RESOLVED FIXED

[Philip Chimento]

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

[Xan Lopez]

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.

[Philip Chimento]

Created attachment 40660 [details]
Profiler output of timing test from sysprof

[Philip Chimento]

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)?

[Krzysztof Kotlenga]

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

[Krzysztof Kotlenga]

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

[Alexander Butenko]

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.

[Alexander Butenko]

ping? 

Patch still applies fine to current 1.1.20. Can it be reviewed?

[Priit Laes (IRC: plaes)]

(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

[Eric Seidel (no email)]

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

[Martin Robinson]

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.

[WebKit Review Bot]

Attachment 49131 [details] did not build on gtk:
Build output: http://webkit-commit-queue.appspot.com/results/290665

[WebKit Review Bot]

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.

[Gustavo Noronha (kov)]

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!

[Martin Robinson]

Committed r55222: <http://trac.webkit.org/changeset/55222>