Bug 791281 – Backport size allocation optimization from master to 3.22

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 791281 - Backport size allocation optimization from master to 3.22


Summary:	Backport size allocation optimization from master to 3.22


Status:	RESOLVED OBSOLETE

Product:	gtk+
Classification:	Platform
Component:	.General
Version:	3.22.x
Hardware:	Other Linux

Importance:	Normal normal
Target Milestone:	---
Assigned To:	gtk-bugs
QA Contact:	gtk-bugs

URL:
Whiteboard:

Depends on:
Blocks:

Reported:	2017-12-05 19:31 UTC by Juan Pablo Ugarte
Modified:	2018-05-02 19:33 UTC

See Also:
GNOME target:	---
GNOME version:	---

Attachments
Test case (1.22 KB, text/x-csrc) 2017-12-05 19:31 UTC, Juan Pablo Ugarte		Details
Proposed patch (13.82 KB, patch) 2017-12-05 19:32 UTC, Juan Pablo Ugarte	none	Details \| Review
Backport of original commit (5.64 KB, patch) 2017-12-05 19:32 UTC, Juan Pablo Ugarte	none	Details \| Review
Squashed original commit + backport (17.99 KB, patch) 2017-12-05 19:34 UTC, Juan Pablo Ugarte	committed	Details \| Review
Proposed pacth (1.51 KB, patch) 2017-12-14 18:38 UTC, Juan Pablo Ugarte	none	Details \| Review
Proposed patch to tix segfault (1.50 KB, patch) 2017-12-14 19:04 UTC, Juan Pablo Ugarte	committed	Details \| Review
Patch for master (1.57 KB, patch) 2017-12-15 13:24 UTC, Juan Pablo Ugarte	committed	Details \| Review

Description Juan Pablo Ugarte 2017-12-05 19:31:08 UTC

Created attachment 365056 [details]
Test case

Gtk 3.22 queue a size allocation if you change the text color with css which make things a lot slower (see test case)

This issue if fixed in master by commit
https://git.gnome.org/browse/gtk+/commit/?id=9bc7581f1c0

Comment 1 Juan Pablo Ugarte 2017-12-05 19:32:09 UTC

Created attachment 365057 [details] [review]
Proposed patch

Original rebased commit

Comment 2 Juan Pablo Ugarte 2017-12-05 19:32:55 UTC

Created attachment 365058 [details] [review]
Backport of original commit

Backport original commit to 3.22

Comment 3 Juan Pablo Ugarte 2017-12-05 19:34:29 UTC

Created attachment 365059 [details] [review]
Squashed original commit + backport

This one is meant for merge the other patch are to make things easier to review

Comment 4 Christian Hergert 2017-12-14 07:23:18 UTC

Commit 492469a94af35c2b102bcf7f07046ef2d09b2b3d is crashing Builder when opening a document.

+ Trace 238258

Thread 1 (Thread 0x7f305b040b00 (LWP 24045))

#0 waitpid
from /lib64/libpthread.so.0
#1 bug_buddy_sigsegv_handler
at ../src/bug-buddy.c line 51
#2 <signal handler called>
#3 gtk_css_style_change_affects
#4 gtk_widget_real_style_updated
at /home/christian/jhbuild/checkout/gtk+-3/gtk/gtkwidget.c line 8173
#5 gtk_text_view_style_updated
at /home/christian/jhbuild/checkout/gtk+-3/gtk/gtktextview.c line 4893
#6 gtk_source_view_style_updated
at /home/christian/jhbuild/checkout/gtksourceview-3/gtksourceview/gtksourceview.c line 4834
#7 ide_source_view_real_style_updated
at ../src/libide/sourceview/ide-source-view.c line 2827
#8 g_cclosure_marshal_VOID__VOID
at /home/christian/jhbuild/checkout/glib/gobject/gmarshal.c line 875
#9 g_type_class_meta_marshal
at /home/christian/jhbuild/checkout/glib/gobject/gclosure.c line 997
#10 g_closure_invoke
at /home/christian/jhbuild/checkout/glib/gobject/gclosure.c line 804
#11 signal_emit_unlocked_R
at /home/christian/jhbuild/checkout/glib/gobject/gsignal.c line 3565
#12 g_signal_emit_valist
at /home/christian/jhbuild/checkout/glib/gobject/gsignal.c line 3391
#13 g_signal_emit
at /home/christian/jhbuild/checkout/glib/gobject/gsignal.c line 3447
#14 _gtk_widget_style_context_invalidated
at /home/christian/jhbuild/checkout/gtk+-3/gtk/gtkwidget.c line 16544
#15 gtk_style_context_real_changed
at /home/christian/jhbuild/checkout/gtk+-3/gtk/gtkstylecontext.c line 190
#16 g_cclosure_marshal_VOID__VOID
at /home/christian/jhbuild/checkout/glib/gobject/gmarshal.c line 875
#17 g_type_class_meta_marshal
at /home/christian/jhbuild/checkout/glib/gobject/gclosure.c line 997
#18 g_closure_invoke
at /home/christian/jhbuild/checkout/glib/gobject/gclosure.c line 804
#19 signal_emit_unlocked_R
at /home/christian/jhbuild/checkout/glib/gobject/gsignal.c line 3565
#20 g_signal_emit_valist
at /home/christian/jhbuild/checkout/glib/gobject/gsignal.c line 3391
#21 g_signal_emit
at /home/christian/jhbuild/checkout/glib/gobject/gsignal.c line 3447
#22 gtk_style_context_validate
at /home/christian/jhbuild/checkout/gtk+-3/gtk/gtkstylecontext.c line 2422
#23 gtk_style_context_invalidate
at /home/christian/jhbuild/checkout/gtk+-3/gtk/gtkstylecontext.c line 2448
#24 _gtk_source_style_scheme_unapply
at /home/christian/jhbuild/checkout/gtksourceview-3/gtksourceview/gtksourcestylescheme.c line 851
#25 gtk_source_view_update_style_scheme
at /home/christian/jhbuild/checkout/gtksourceview-3/gtksourceview/gtksourceview.c line 4813
#26 buffer_style_scheme_changed_cb
at /home/christian/jhbuild/checkout/gtksourceview-3/gtksourceview/gtksourceview.c line 1528
#27 g_cclosure_marshal_VOID__PARAM
at /home/christian/jhbuild/checkout/glib/gobject/gmarshal.c line 1832
#28 g_closure_invoke
at /home/christian/jhbuild/checkout/glib/gobject/gclosure.c line 804
#29 signal_emit_unlocked_R
at /home/christian/jhbuild/checkout/glib/gobject/gsignal.c line 3635
#30 g_signal_emit_valist
at /home/christian/jhbuild/checkout/glib/gobject/gsignal.c line 3391
#31 g_signal_emit
at /home/christian/jhbuild/checkout/glib/gobject/gsignal.c line 3447
#32 g_object_dispatch_properties_changed
at /home/christian/jhbuild/checkout/glib/gobject/gobject.c line 1080
#33 g_object_notify_queue_thaw
at /home/christian/jhbuild/checkout/glib/gobject/gobject.c line 296
#34 g_object_setv
at /home/christian/jhbuild/checkout/glib/gobject/gobject.c line 2248
#35 g_object_set_property
at /home/christian/jhbuild/checkout/glib/gobject/gobject.c line 2529
#36 g_settings_binding_key_changed
at /home/christian/jhbuild/checkout/glib/gio/gsettings.c line 2671
#37 g_settings_bind_with_mapping
at /home/christian/jhbuild/checkout/glib/gio/gsettings.c line 2968
#38 g_settings_bind
at /home/christian/jhbuild/checkout/glib/gio/gsettings.c line 2796
#39 _ide_editor_view_init_settings
at ../src/libide/editor/ide-editor-view-settings.c line 171
#40 ide_editor_view_constructed
at ../src/libide/editor/ide-editor-view.c line 678
#41 g_object_new_internal
at /home/christian/jhbuild/checkout/glib/gobject/gobject.c line 1837
#42 g_object_new_valist
at /home/christian/jhbuild/checkout/glib/gobject/gobject.c line 2120
#43 g_object_new
at /home/christian/jhbuild/checkout/glib/gobject/gobject.c line 1640
#44 ide_editor_perspective_focus_buffer
at ../src/libide/editor/ide-editor-perspective.c line 612
#45 ide_editor_workbench_addin_on_load_buffer
at ../src/libide/editor/ide-editor-workbench-addin.c line 96
#46 ffi_call_unix64
from /lib64/libffi.so.6
#47 ffi_call
from /lib64/libffi.so.6
#48 g_cclosure_marshal_generic_va
at /home/christian/jhbuild/checkout/glib/gobject/gclosure.c line 1604
#49 _g_closure_invoke_va
at /home/christian/jhbuild/checkout/glib/gobject/gclosure.c line 867
#50 g_signal_emit_valist
at /home/christian/jhbuild/checkout/glib/gobject/gsignal.c line 3300
#51 g_signal_emit
at /home/christian/jhbuild/checkout/glib/gobject/gsignal.c line 3447
#52 ide_buffer_manager__load_file_query_info_cb
at ../src/libide/buffers/ide-buffer-manager.c line 724
#53 g_task_return_now
at /home/christian/jhbuild/checkout/glib/gio/gtask.c line 1148
#54 complete_in_idle_cb
at /home/christian/jhbuild/checkout/glib/gio/gtask.c line 1162
#55 g_idle_dispatch
at /home/christian/jhbuild/checkout/glib/glib/gmain.c line 5553
#56 g_main_dispatch
at /home/christian/jhbuild/checkout/glib/glib/gmain.c line 3183
#57 g_main_context_dispatch
at /home/christian/jhbuild/checkout/glib/glib/gmain.c line 3848
#58 g_main_context_iterate
at /home/christian/jhbuild/checkout/glib/glib/gmain.c line 3921
#59 g_main_context_iteration
at /home/christian/jhbuild/checkout/glib/glib/gmain.c line 3982
#60 g_application_run
at /home/christian/jhbuild/checkout/glib/gio/gapplication.c line 2402
#61 main
at ../src/main.c line 154

Comment 5 Juan Pablo Ugarte 2017-12-14 18:38:28 UTC

Created attachment 365555 [details] [review]
Proposed pacth

Comment 6 Juan Pablo Ugarte 2017-12-14 19:04:18 UTC

Created attachment 365557 [details] [review]
Proposed patch to tix segfault

Comment 7 Emmanuele Bassi (:ebassi) 2017-12-14 20:36:49 UTC

Review of attachment 365557 [details] [review]:

Looks good to me.

Comment 8 Christian Hergert 2017-12-14 22:21:04 UTC

Can verify that the patch fixes the segfault.

Comment 9 Juan Pablo Ugarte 2017-12-15 13:24:12 UTC

Created attachment 365585 [details] [review]
Patch for master

We should also apply this to master

Comment 10 Juan Pablo Ugarte 2017-12-15 13:30:25 UTC

Review of attachment 365585 [details] [review]:

Pushed to master

Comment 11 Daniel Boles 2017-12-17 19:41:58 UTC

For the record, this was reverted in gtk-3-22 due to reports of missing invalidations:
  https://git.gnome.org/browse/gtk+/commit/?h=gtk-3-22&id=32332b3a035d7b5781b6b9eb0f13778ecda3942b

Comment 12 Emmanuele Bassi (:ebassi) 2017-12-17 23:57:33 UTC

Given that it was reverted, let's reopen this issue for further investigation.

Comment 13 Christian Hergert 2017-12-18 05:18:25 UTC

I wouldn't be surprised if there is quite a bit of random code that isn't doing gtk_widget_queue_draw() in custom size_allocate handlers. In fact this is probably an issue I've run into with Dazzle. So in that case, Dazzle is certainly to blame.

Comment 14 Christian Hergert 2018-01-09 23:47:19 UTC

> <xjuan> so about bug 791281, do you know if the regression was in gtk or was it in dazzle?

I don't think it was Dazzle, because even after some tweaks on my side, I was still seeing regressions.

Comment 15 GNOME Infrastructure Team 2018-05-02 19:33:24 UTC

-- GitLab Migration Automatic Message --

This bug has been migrated to GNOME's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/gtk/issues/988.