Bug 37836 - Unneeded invocations of memcpy()
Summary: Unneeded invocations of memcpy()
Status: RESOLVED FIXED
Alias: None
Product: cairo
Classification: Unclassified
Component: general (show other bugs)
Version: 1.10.3
Hardware: x86 (IA32) All
: medium normal
Assignee: Carl Worth
QA Contact: cairo-bugs mailing list
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-06-01 14:37 UTC by Steve Snyder
Modified: 2011-09-16 08:15 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:


Attachments
Skip zero-byte-length call to memcpy() (534 bytes, patch)
2011-06-01 14:37 UTC, Steve Snyder
Details | Splinter Review

Description Steve Snyder 2011-06-01 14:37:54 UTC
Created attachment 47444 [details] [review]
Skip zero-byte-length call to memcpy()

In function _cairo_path_fixed_add() (file cairo-path-fixed.c, as pulled from the Thunderbird 5.0 Beta1 source) memcpy() is frequently invoked to copy zero bytes.

My observation with TB 5.0b1 is that in 16% of the calls a byte length of zero is specified.  Further, the code as built by Mozilla using VS2005/VS2008 is generated as functions, not as inline intrinsics.

Not a huge burden, but why incur the cost of these pointless calls to memcpy() if we don't have to?
Comment 1 Behdad Esfahbod 2011-06-01 14:55:39 UTC
I'd patch _cairo_path_buf_add_points() instead.
Comment 2 Steve Snyder 2011-06-01 15:06:09 UTC
(In reply to comment #1)
> I'd patch _cairo_path_buf_add_points() instead.

But the entirety of _cairo_path_buf_add_points() (which is little more than a wrapper around memcpy) is pointless if the num_points variable is zero.
Comment 3 Behdad Esfahbod 2011-06-01 15:15:14 UTC
Sure, but why should the caller bother checking when the callee can do?
Comment 4 Chris Wilson 2011-09-16 08:15:05 UTC
commit 1ce5d4707cf260618bd4d61f39aad4371ffa3336
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date:   Fri Sep 16 16:11:42 2011 +0100

    path: Skip calls to zero-length memcpy
    
    We attempt to copy 0 points onto the array of path points for a
    close-path. This is pointless and an unnecessary function call under
    MSVC at least.
    
    Based on a patch by Steve Snyder, incorporating Behdad's review
    comments.
    
    Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=37836
    Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.