Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(1010)

Issue 551690046: Shortcut Rational addition if either operand is zero (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
4 months ago by hanwenn
Modified:
3 months ago
Reviewers:
Dan Eble, lemzwerg, hahnjo
CC:
lilypond-devel_gnu.org
Visibility:
Public.

Description

Shortcut Rational addition if either operand is zero In a GProf profile of the Carver MSDM score, before 3.14% of cumulative time was from Rational::operator+=(). Afterwards, it represents 1.02% of cumulative time.

Patch Set 1 #

Total comments: 5

Patch Set 2 : dan's comments #

Patch Set 3 : reorder (oops, forgot to upload) #

Unified diffs Side-by-side diffs Delta from patch set Stats (+19 lines, -1 line) Patch
M flower/rational.cc View 1 2 1 chunk +5 lines, -1 line 0 comments Download
M flower/test-rational.cc View 1 1 chunk +14 lines, -0 lines 0 comments Download

Messages

Total messages: 9
lemzwerg
LGTM. Maybe you can add a comment w.r.t. speed improvement.
4 months ago (2020-04-12 12:40:23 UTC) #1
Dan Eble
LGTM https://codereview.appspot.com/551690046/diff/573690043/flower/rational.cc File flower/rational.cc (right): https://codereview.appspot.com/551690046/diff/573690043/flower/rational.cc#newcode257 flower/rational.cc:257: if (is_infinity ()) The hazard of reviewing this ...
4 months ago (2020-04-12 16:55:06 UTC) #2
hanwenn
dan's comments
4 months ago (2020-04-12 18:00:35 UTC) #3
hanwenn
https://codereview.appspot.com/551690046/diff/573690043/flower/rational.cc File flower/rational.cc (right): https://codereview.appspot.com/551690046/diff/573690043/flower/rational.cc#newcode257 flower/rational.cc:257: if (is_infinity ()) On 2020/04/12 16:55:05, Dan Eble wrote: ...
4 months ago (2020-04-12 18:01:31 UTC) #4
hanwenn
reorder (oops, forgot to upload)
3 months, 3 weeks ago (2020-04-17 09:15:14 UTC) #5
hanwenn
I pushed this to staging by accident; sorry. I apparently have no permission to do ...
3 months, 3 weeks ago (2020-04-17 09:25:03 UTC) #6
hahnjo
On 2020/04/17 09:25:03, hanwenn wrote: > I pushed this to staging by accident; sorry. > ...
3 months, 3 weeks ago (2020-04-17 09:49:18 UTC) #7
hanwenn
Oh, you are right. Thanks for checking! On Fri, Apr 17, 2020 at 11:49 AM ...
3 months, 3 weeks ago (2020-04-17 09:58:34 UTC) #8
hanwenn
3 months ago (2020-05-09 10:41:54 UTC) #9
commit fb98d6be7ed06969a8eeab7b974e8734d8cd21b1
Author: Han-Wen Nienhuys <hanwen@lilypond.org>
Date:   Sun Apr 12 13:32:01 2020 +0200

    Shortcut Rational addition if either operand is zero
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b