| Summary: | Compiling breeze-icons-6.9 exhausts memory on 32-bit hosts | ||
|---|---|---|---|
| Product: | [Plasma] Breeze | Reporter: | Dave Flogeras <dflogeras2> |
| Component: | Icons | Assignee: | visual-design |
| Status: | RESOLVED FIXED | ||
| Severity: | major | CC: | christoph, dflogeras2, duha.bugs, kainz.a, m, nate, rikmills, sam |
| Priority: | HI | ||
| Version First Reported In: | unspecified | ||
| Target Milestone: | --- | ||
| Platform: | Other | ||
| OS: | Linux | ||
| See Also: | https://bugs.gentoo.org/show_bug.cgi?id=956679 | ||
| Latest Commit: | https://invent.kde.org/frameworks/breeze-icons/-/commit/ddac172cba33db77143b9df4e5ad32be4e19cff6 | Version Fixed In: | |
| Sentry Crash Report: | |||
|
Description
Dave Flogeras
2025-02-08 10:41:58 UTC
We did use in the past the big resource way to add that, but that did lead to issues with LTO and Co. Do you think it is worth raising with GCC folks? In my view, at the end of the day, it's compiling ~100M of bytes not C code. This may be overly naive, but I'm not sure why it would consume so much RAM when there's nothing to optimize here. I did try make -j1, with -O0 to no avail here (my Gentoo system default is -O2). I did try playing with this with a user and messing with GC and a few other things and it didn't make a difference. I think the proper way of doing it would be to use #embed, I guess. We had another report of this today in Gentoo. (In reply to Christoph Cullmann from comment #1) > We did use in the past the big resource way to add that, but that did lead > to issues with LTO and Co. It should work if we disable LTO (just pass -fno-lto) to the one TU containing the resources. *** Bug 505503 has been marked as a duplicate of this bug. *** This did not occur in Ubuntu 32 bit builds until frameworks 6.15.0, and was triggered by the addition of the commit: https://invent.kde.org/frameworks/breeze-icons/-/commit/135e59fb4395c1779a52ab113cc70f7baa53fd5d I will be forced to revert that commit in packaging, which is very sub optimal. Christoph, would you mind looking into this? What happens if one turns off optimizations for that package? One can try to revert https://invent.kde.org/frameworks/breeze-icons/-/merge_requests/379 for 32 bit ops, too, but that was done as distributions complained it is broken but nobody fixes it upstream in Qt. We can revert below request to fix it if distros stop to LTO that framework. I could try to add a unit test to check if icons are there in the lib. But that needs distro feedback, the current state is the result of that. BTW., which Qt version is used for the compile? (In reply to Christoph Cullmann from comment #10) > BTW., which Qt version is used for the compile? Currently 6.8.3 in Ubuntu 25.10 dev release. We will hopefully get moving to 6.9.x once Plasma 6.4 is properly landed. Reverting MR 397 and disabling LTO results in a successful build. MR 379 I mean If I add a merge request for that with a test that the lib contains the right stuff, would you test that for me? (In reply to Christoph Cullmann from comment #14) > If I add a merge request for that with a test that the lib contains the > right stuff, would you test that for me? Absolutely :) Ok A possibly relevant merge request was started @ https://invent.kde.org/frameworks/breeze-icons/-/merge_requests/481 See the request, uses again the big/binary resource variant and adds a test that checks if files got lost, like we had in bug 487452. Tested MR 481 with and without LTO disabled. With LTO disabled, builds and tests succeeded on all 6 architectures. With LTO enabled builds succeeded but the test failed, as expected. Exception was ironically armhf, where the test passed. Go figure! (note: I had to adjust packaging to add a virtual dispaply in the buildd test env with xvfb to get the new test to run, but that is not a big issue) (note: I had to adjust packaging to add a virtual dispaply in the buildd test env with xvfb to get the new test to run, but that is not a big issue) That I can fix, too. Can you retry? I think QTEST_GUILESS_MAIN should be good enough for that test. If that works, somebody must approve it and one needs to inform the packagers about the LTO issue. *** Bug 487452 has been marked as a duplicate of this bug. *** (In reply to Christoph Cullmann from comment #21) > Can you retry? > > I think QTEST_GUILESS_MAIN should be good enough for that test. Seems so. Run and passed as before. Thank you! Thanks for testing. Can somebody approve the merge or shall we keep it as is? (In reply to Christoph Cullmann from comment #26) > Can somebody approve the merge or shall we keep it as is? Approved it, as it is not good that current shipped code is not buildable on some architectures. Additionally I conformed in a debian 32 bit env that this will hit Debian on i386 and armhf when they get to 6.15.0 after they unfreeze from trixie release freeze. (In reply to Rik Mills from comment #27) > (In reply to Christoph Cullmann from comment #26) > > Can somebody approve the merge or shall we keep it as is? > > Approved it, as it is not good that current shipped code is not buildable on > some architectures. Additionally I conformed in a debian 32 bit env that > this will hit Debian on i386 and armhf when they get to 6.15.0 after they > unfreeze from trixie release freeze. Is there some distributor list to ping about the change? Git commit 75fd5c16d1cf0bb6756d4dedd073fed2ef6ee325 by Christoph Cullmann. Committed on 17/06/2025 at 16:42. Pushed by cullmann into branch 'master'. use big resource variant, else we OOM on 32 bit machines this has issues with LTO, we have a unit test that tells if stuff got lost in the DLL fix the resource loading for static library building, uncovered by the new unit test Related: bug 487452 M +1 -0 .gitignore M +5 -1 autotests/CMakeLists.txt A +68 -0 autotests/resourcetest.cpp [License: LGPL(v2.0+)] M +3 -2 src/lib/CMakeLists.txt M +9 -0 src/lib/breezeicons.cpp https://invent.kde.org/frameworks/breeze-icons/-/commit/75fd5c16d1cf0bb6756d4dedd073fed2ef6ee325 > Is there some distributor list to ping about the change? https://mail.kde.org/mailman/listinfo/distributions Ok, send to that list this mail: Hi, Breeze Icons resource building got changed back to big resource and a unit test was added to check if stuff is missing. You might need to turn off LTO. See https://invent.kde.org/frameworks/breeze-icons/-/merge_requests/481 and linked bugs for details. Greetings Christoph Will be in moderation queue first, but guess will then convey enough info. In any case it is good we have a test, the static build was always broken. Git commit ddac172cba33db77143b9df4e5ad32be4e19cff6 by Sam James. Committed on 19/06/2025 at 21:22. Pushed by cullmann into branch 'master'. Disable LTO for qt_add_big_resources In fa44b11bc2b36d5c3cfc5c3403ea75b2fff57253, we went from qt_add_big_resources to qt_add_resources because the former breaks with LTO, then we recently went back to qt_add_big_resources in 75fd5c16d1cf0bb6756d4dedd073fed2ef6ee325 to fix the build on 32-bit machines. That leaves LTO broken: try to use CMake to disable LTO for kbreezeicons rather than asking distributors to disable LTO manually. I tried to use CMake's support for LTO/IPO to disable it per-target but it doesn't actually pass -fno-lto or anything if you're not using CMake's support to enable LTO in the first place. Bug: https://bugs.gentoo.org/956679 Bug: https://bugreports.qt.io/browse/QTBUG-41301 Related: bug 487452 M +11 -0 CMakeLists.txt https://invent.kde.org/frameworks/breeze-icons/-/commit/ddac172cba33db77143b9df4e5ad32be4e19cff6 |