Bug 15798 - Improved performance for Summary group primitives
Summary: Improved performance for Summary group primitives
Status: CLOSED FIXED
Alias: None
Product: R
Classification: Unclassified
Component: Low-level (show other bugs)
Version: R-devel (trunk)
Hardware: Other Linux
: P5 enhancement
Assignee: R-core
URL:
Depends on:
Blocks:
 
Reported: 2014-05-08 22:39 UTC by Michael Lawrence
Modified: 2015-12-14 13:45 UTC (History)
1 user (show)

See Also:


Attachments
Patch for na.rm performance in Summary group (4.13 KB, patch)
2014-05-08 22:39 UTC, Michael Lawrence
Details | Diff
Simpler shallow duplication strategy (1.11 KB, patch)
2014-05-08 23:13 UTC, Michael Lawrence
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Lawrence 2014-05-08 22:39:22 UTC
Created attachment 1601 [details]
Patch for na.rm performance in Summary group

There was a performance issue identified here:
http://stackoverflow.com/questions/23530915/in-r-why-is-sum-so-slow-compared-to-others-such-as-cumsum/23531890#23531890

Thanks to Martin Morgan, the bottleneck was identified to be duplication of the call in preparation for dispatch. The attached patch resolves the issue by avoiding duplication when not necessary. It also avoids adding the na.rm argument to the arguments, because it seems undesirable to pass the argument to a method when the user did not specify it; missing() returns a confusing result.

The patch is only lightly tested, but it does improve performance by orders of magnitude for small inputs.
Comment 1 Michael Lawrence 2014-05-08 23:13:19 UTC
Created attachment 1602 [details]
Simpler shallow duplication strategy
Comment 2 Brian Ripley 2014-05-12 17:34:41 UTC
It is unclear what the intention here is: the two patches conflict.

The second has been applied in R-devel.
Comment 3 Martin Maechler 2015-10-08 13:24:35 UTC
As the 2nd patch has been applied (16 months) ago, and there was no follow up,
this can be considered fixed.