After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 785313 - [Windows 10] On first start, meld eats 100% CPU for a few minutes
[Windows 10] On first start, meld eats 100% CPU for a few minutes
Status: RESOLVED FIXED
Product: meld
Classification: Other
Component: general
3.16.x
Other Windows
: Normal normal
: ---
Assigned To: meld-maint
meld-maint
Depends on:
Blocks:
 
 
Reported: 2017-07-23 13:44 UTC by Christian Stadelmann
Modified: 2017-10-25 20:55 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
fix by switching to native win32 fonts instead of fontconfig (1.04 KB, patch)
2017-10-12 14:16 UTC, Vasily Galkin
none Details | Review
fix by switching to native win32 fonts instead of fontconfig (2.16 KB, patch)
2017-10-25 17:01 UTC, Vasily Galkin
none Details | Review

Description Christian Stadelmann 2017-07-23 13:44:58 UTC
Steps to reproduce:
1. on Windows 10,
2. install latest version of meld (3.16.2) from meldmerge.org
3. run meld

What happens:
Meld takes very long before starting for the first time. It runs at 100% CPU load on one core.

What should happen:
Show the window sooner, don't busy loop the CPU when there is nothing to do.

Additional info:
On subsequent runs, meld behaves as expected
Comment 1 Kai Willadsen 2017-08-01 21:12:24 UTC
I only had it take about 15 seconds, but even that's not good. I'm almost certain that this is also new in Windows 10; I feel like I would have noticed this on Windows 7.

Is there any chance that this is e.g., Microsoft doing binary signature checking or something? I can't think what we'd be doing that would cause this.
Comment 2 Vasily Galkin 2017-10-05 23:17:39 UTC
Observed the same behaviour while trying to debug installer on new windows 10 machine.

100% of one cpu core was used for several minutes.
As far as I remember several years ago gtk on windows on the first run started a subprocess that performs some caching of all system font data and this takes near 2 minutes. After this subprocess termination meld window appeared. AFter some version such subprocess disappeared, but maybe the caching is just made inside meld process automatically.
Comment 3 Vasily Galkin 2017-10-11 10:01:06 UTC
More specifically it is fontconfig DLL spending a lot of time in creating cache in   %LOCALAPPDATA%\fontconfig\cache

See https://bugs.freedesktop.org/show_bug.cgi?id=64766 for reasons discussion.

The current in-development version of fontconfig has some speedup in this area in master branch: 

https://github.com/behdad/fontconfig/commit/c524522bb45f71dfeaa8fd1ec277537dd6e85afa

So until it is released speedup can't be achieved but a sligtly better user experience can: an initial fontconfig cache can be built during meld installation, by using either the fc-cache.exe binary or by trying to run meld to the step where gtk or something is initialized.

Also with default config in "C:\Program Files (x86)\Meld\etc\fonts\" the cache created and used by fontconfig is in user-specific folder, so this helps only a user that executed meld installer.

Placing cache to a system-wide location like C:\Program Files (x86)\Meld\var\cache\fontconfig is possible by adding cache file setting to "/var/cache/fontconfig", but the exact consequences in a case when fonts are updated are not clear, since that folder wouldn't be writeable for meld process.

Another variant is placing in all-writeable directory in %ALLUSERSPROFILE%\Meld\var\cache\fontconfig by specifiing the full path in config but this requires ensuring setting correct rights for the folder during installation.
Comment 4 Vasily Galkin 2017-10-12 14:16:35 UTC
Created attachment 361424 [details] [review]
fix by switching to native win32 fonts instead of fontconfig

It turns out that pango backend can be swicthed in runtime.
Attaching patch that forces win32 font handling instead of fontconfig on win32 platform.
Comment 5 Kai Willadsen 2017-10-20 22:00:47 UTC
Review of attachment 361424 [details] [review]:

Wow, thanks for chasing that down! After reading through those rather epic bug reports, I agree that this is our best option.

Minor comments below, but basically this looks good to me!

::: bin/meld
@@ +355,3 @@
+    if sys.platform == 'win32':
+        #FontConfig on win32 at least with version <= 2.12.6 can cause several
+        #minutes 'hang' during first startup. So use native fonts backend.

Could you please PEP8 these comments? Specifically, there should be a space after the '#' that starts the lines.

@@ +357,3 @@
+        #minutes 'hang' during first startup. So use native fonts backend.
+        import gi
+        gi.require_version("PangoCairo", "1.0")

I think for consistency we should move this to `check_requirements()` where we do other checks like this. I know that technically it's only for the Windows port, but I can imagine that we might want this for OSX as well, and in reality we already require Pango and Cairo individually, so I don't feel bad about requiring this import everywhere.
Comment 6 Kai Willadsen 2017-10-20 22:00:49 UTC
Review of attachment 361424 [details] [review]:

Wow, thanks for chasing that down! After reading through those rather epic bug reports, I agree that this is our best option.

Minor comments below, but basically this looks good to me!

::: bin/meld
@@ +355,3 @@
+    if sys.platform == 'win32':
+        #FontConfig on win32 at least with version <= 2.12.6 can cause several
+        #minutes 'hang' during first startup. So use native fonts backend.

Could you please PEP8 these comments? Specifically, there should be a space after the '#' that starts the lines.

@@ +357,3 @@
+        #minutes 'hang' during first startup. So use native fonts backend.
+        import gi
+        gi.require_version("PangoCairo", "1.0")

I think for consistency we should move this to `check_requirements()` where we do other checks like this. I know that technically it's only for the Windows port, but I can imagine that we might want this for OSX as well, and in reality we already require Pango and Cairo individually, so I don't feel bad about requiring this import everywhere.
Comment 7 Vasily Galkin 2017-10-25 17:01:12 UTC
Created attachment 362277 [details] [review]
fix by switching to native win32 fonts instead of fontconfig

The requirement on pangocairo version was only for silencing

>PyGIWarning: PangoCairo was imported without specifying a version first. Use gi.require_version('PangoCairo', '1.0') before import to ensure that the right version gets loaded.

(and to make code more robust to future possible changing of namespace version)

In fact I think any version from last 8 years would handle the call.
But for the simplicity and consistency with other imports an explicit quite old version is mentioned as a requirement.
Comment 8 Vasily Galkin 2017-10-25 17:04:16 UTC
Also I noticed that other checks doesn't catch ValueError that can be raised by a gi.require_version

Note that PangoCairo catches ValueError but not AssertionError since there is no asserts.
Comment 9 Kai Willadsen 2017-10-25 20:55:49 UTC
(In reply to Vasily Galkin from comment #7)
> The requirement on pangocairo version was only for silencing

Yep, this looks good to me, thanks! I've just pushed this to current master.


(In reply to Vasily Galkin from comment #8)
> Also I noticed that other checks doesn't catch ValueError that can be raised
> by a gi.require_version

Good catch. I've just fixed this, again in master.

When we start doing the binary builds again (I'm sorry, I *will* try to get to reviewing your changes soon) then we should probably look to cherry-pick these back to the stable branch.