Created attachment 424440 [details] [diff] [review] remove functions There are a number of unused functions in the cairo library. Removing them results in an admittedly modest decrease of 1500 bytes from libxul. I was told that it may be more appropriate to #ifdef 0 unused code in any C libraries but I've posted a patch with the functions removed. I suppose it may make more sense to file this sort of stuff upstream but I'm looking for the community's input since this is quite likely not the only dead C code in the tree. Functions removed: cairo-cache.c: _cairo_hash_bytes cairo-path-fixed.c: _cairo_path_fixed_hash _cairo_path_fixed_size _cairo_path_fixed_equal cairo-pattern.c: _cairo_solid_pattern_hash _cairo_gradient_color_stops_hash _cairo_linear_pattern_hash _cairo_radial_pattern_hash _cairo_surface_pattern_hash _cairo_pattern_hash _cairo_gradient_pattern_color_stops_size _cairo_pattern_size _cairo_solid_pattern_equal _cairo_gradient_color_stops_equal _cairo_linear_pattern_equal _cairo_radial_pattern_equal _cairo_surface_pattern_equal _cairo_pattern_equal
Comment on attachment 424440 [details] [diff] [review] remove functions I'll let jeff make the final call here, but I don't think we should do this unless this patch gets applied to upstream cairo as well -- otherwise tracking merges will be painful for not much good reason.
Hrm, if this is unused code how come it's taking so much space in libxul? In theory shouldn't the linker notice that the code is not exposed externally and not called internally, and rip all of it out of the library? With the right linker settings in theory dead code elimination should take care of this.
Comment on attachment 424440 [details] [diff] [review] remove functions Indeed, merging this would make tracking cairo upstream harder than it needs to be. Further, removing unused functions is the linker's job.
(In reply to comment #1) > (From update of attachment 424440 [details] [diff] [review]) > I'll let jeff make the final call here, but I don't think we should do this > unless this patch gets applied to upstream cairo as well -- otherwise tracking > merges will be painful for not much good reason. Indeed, we use -dead_strip on OS X which does this. I assume the win32 linker does the right thing too. On Linux, (which Ehren was testing on?) we could/should use '-ffunction-sections -Wl,-gc-sections'
> we could/should use '-ffunction-sections -Wl,-gc-sections' Actually bug 537857 has already been filed for this. Is '-ffunction-sections -Wl,-gc-sections' smart enough to make these removals though? Just to test I ran a few comparison builds on rev 49cc24d8bd6b and compared the size of libxul with the following results (I'm on Linux x86_64 btw): using -ffunction-sections -Wl,-gc-sections: with the removal patch: 29079688 bytes without: 29082888 bytes difference: 3200 bytes ---- without using -ffunction-sections -Wl,-gc-sections: with the patch: 31050385 bytes without: 31051889 bytes difference: 1504 bytes ---- I also tried '-fdata-sections -ffunction-sections -Wl,-gc-sections': with the path: 29028476 bytes without: 29033244 bytes difference: 4768 bytes Odd that the disparity widens with these flags... I completely understand that this would be a tracking nightmare just to save 1 or 2 k though
If you run 'nm' on the resulting binary you should be able to find out whether these symbols included or not.
Here's what I found with nm: 0000000000d89580 t _cairo_pattern_equal 0000000000d894d0 t _cairo_gradient_color_stops_equal 0000000000d89770 t _cairo_pattern_hash 0000000000d89050 t _cairo_pattern_size 0000000000d89700 t _cairo_gradient_color_stops_hash 0000000000d85040 t _cairo_path_fixed_equal 0000000000d84730 t _cairo_path_fixed_size 0000000000d85bf0 t _cairo_path_fixed_hash 0000000000dbe2a0 t _cairo_hash_bytes These were found in the libxul compiled with '-fdata-sections -ffunction-sections -Wl,-gc-sections'
Mark as WONTFIX or send the patch to Cairo?
Please file a new bug about the unused functions not being stripped.