GNOME Bugzilla – Bug 790270
avoid copy of CSS data from resources
Last modified: 2018-02-05 17:23:32 UTC
When loading CSS from resources, we copy the data onto the heap before parsing it. This is unnecessary.
Created attachment 363461 [details] [review] avoid a copy of data whenever resources are used
Review of attachment 363461 [details] [review]: ::: gtk/gtkcssprovider.c @@ +1886,3 @@ g_return_val_if_fail (GTK_IS_CSS_PROVIDER (css_provider), FALSE); g_return_val_if_fail (G_IS_FILE (file), FALSE); This should go into gtk_css_provider_load_internal(), probably right about https://git.gnome.org/browse/gtk+/tree/gtk/gtkcssprovider.c?id=3.22.26#n1741. @@ +1889,3 @@ + uri = g_file_get_uri (file); + + if (g_str_has_prefix (uri, "resource://")) g_file_has_uri_scheme(). See https://git.gnome.org/browse/gtk+/tree/gtk/gtkcssimageurl.c?id=3.22.26#n41 for an example of how to special-case resource loading inside the GTK code base. @@ +1891,3 @@ + if (g_str_has_prefix (uri, "resource://")) + { + const gchar *path = uri + strlen ("resource://"); This needs g_uri_unescape_string().
Created attachment 363464 [details] [review] Check for resource in load_internal to ensure maximal use
Created attachment 363466 [details] [review] utils: add gtk_file_load_bytes() helper This helper will load GBytes for a GFile, but try to reuse the embedded data for a gresource to reduce the chances of copying data to the heap.
Created attachment 363467 [details] [review] css: avoid copying resource data To avoid copying data from gresources to the heap, we can use the newly added gtk_file_load_bytes(). That function will check for resource:// URIs and access their internal data directly. Other URI schemes will read the contents into memory and return a GBytes as normal.
Comment on attachment 363467 [details] [review] css: avoid copying resource data This can go into gtk-3-22 now. Not sure if we should push it into master or make it block on the glib bug about g_file_load_bytes() first.
Cool, I'll leave this bug open so we can think about 4.x. I think so that we don't forget about this, maybe we should apply to 4.x now, and just change the call once we get the API in glib. Attachment 363466 [details] pushed as d46c072 - utils: add gtk_file_load_bytes() helper Attachment 363467 [details] pushed as b654130 - css: avoid copying resource data
Do we have any resolution for this?
I haven't done anything on this since the 3.22 patches. It would be nice if someone more familiar with the 4.x code base could do it. I did land the g_file_load_bytes() in glib though. https://git.gnome.org/browse/glib/commit/?id=2227918dfd0bd13d52191538016e88f1344cc196
I don't think we want this in GTK 3.22 because it would require upping the glib dependency for g_file_load_bytes(), so I'm closing this.
It already is in gtk-3-22, but not in master.
One difference is that the 3-22 version doesn't check whether the bytes is null before unrefing, which may cause noisier output than needed if the load fails.