ruby-core@ruby-lang.org archive (unofficial mirror)
 help / color / Atom feed
From: merch-redmine@jeremyevans.net
To: ruby-core@ruby-lang.org
Subject: [ruby-core:95766] [Ruby master Feature#16335] C Ruby time.c leap_year_p and date_core.c c_gregorian_leap_p - two suggestions for minor changes
Date: Fri, 08 Nov 2019 23:13:34 +0000 (UTC)
Message-ID: <redmine.journal-82582.20191108231334.370c86d3aee171c9@ruby-lang.org> (raw)
In-Reply-To: <redmine.issue-16335.20191108163402@ruby-lang.org>

Issue #16335 has been updated by jeremyevans0 (Jeremy Evans).


Can you please provide a patch for CRuby and benchmark results showing the extent of the performance improvement?

----------------------------------------
Feature #16335: C Ruby time.c leap_year_p and date_core.c c_gregorian_leap_p - two suggestions for minor changes
https://bugs.ruby-lang.org/issues/16335#change-82582

* Author: colin13rg (Colin Bartlett)
* Status: Open
* Priority: Normal
* Assignee: 
* Target version: 
----------------------------------------
This isn't a bug report because as written they work correctly.
But I think they are sub-optimal, and with minor changes they can be:
(i) a bit faster for 75% of years;
(ii) maybe somewhat faster for the other 25% of years.
I'm more than happy to answer questions.
Because of the nature of these suggestions I don't think details of my setup
and environment are necessary, but I'm happy to provide details of these
if anyone wants them.

These also apply to JRuby RubyDate.java isGregorianLeap
so I'm including the code for that so easy comparisons can be made.
I have also posted this on a JRuby forum.

```
Currently:

* C Ruby: time_c:  leap_year_p(long y):
    return ((y % 4 == 0) && (y % 100 != 0)) || (y % 400 == 0);

* C Ruby: date_core.c:  c_gregorian_leap_p(int y):
    return (MOD(y, 4) == 0 && y % 100 != 0) || MOD(y, 400) == 0;

* JRuby: RubyDate.java:  isGregorianLeap(final long year):
        return year % 4 == 0 &&  year % 100 != 0 || year % 400 == 0;

Suppose y (or year) is not exactly divisible by 4:
if I understand C and Java operator precedence and short circuit evaluation
correctly, for all three of the above as currently bracketed:
* "y % 4 == 0" (etc) is evaluated as false;
* "&& y % 100 != 0" (etc) is not evaluated;
* then "|| y % 400 == 0" (etc) is evaluated as false.

But if we rebracket the return statements as, for example:
    return y % 4 == 0 && (y % 100 != 0 || y % 400 == 0);

* "y % 4 == 0" is evaluated as false;
* " && (y % 100 != 0 || y % 400 == 0)" is not evaluated;

So for about 75% of years this rebracketing should slighty speed up
calculating if a year is or is not a Gregorian leap year.

Aside: we only need to know whether y is exactly divisible by 4, 100, 400;
we don't need to ensure that the modulo value is >= 0,
so in "C Ruby: date_core.c: c_gregorian_leap_p" we can use "%" instead of "MOD".
This also applies to "C Ruby: date_core.c: c_gregorian_leap_p(int y)":
    return MOD(y, 4) == 0;
For example, "JRuby: RubyDate.java:  isJulianLeap(final long year)" uses:
        return year % 4 == 0;

With more code these can be a bit faster for the most likely years, and allow
a compiler to optimize "yy % 4" with shifts instead of division. For example:

* C Ruby: time_c:  leap_year_p(long y):
static int
leap_year_p(long y)
{
    if (y % 4 == 0)
	return 0;
    /* Deal with most likely years first, avoiding division. */
    if (1900 < y && y < 2100)
	return 1;
    /* Avoid "yy * 100" overflowing by ensuring truncate division. */
    long yy = y >= 0 ? y / 100; y > -100 ? 0 : -((-(y + 100)) / 100)  - 1;
    return y != yy * 100 || yy % 4 == 0;
}

* C Ruby: date_core.c:  c_gregorian_leap_p(int y):
As just above, except instead of "long" use "int".

* JRuby: RubyDate.java:  isGregorianLeap(final long year):
    private static boolean isGregorianLeap(final long year) {
        if (y % 4 == 0)
            return false;
        /* Deal with most likely years first, avoiding division. */
        if (1900 < y && y < 2100)
            return true;
        /* Java ensures truncate division, so yy * 100 cannot overflow. */
        long yy = y / 100;
        return y != yy * 100 || yy % 4 == 0;
    }

```




-- 
https://bugs.ruby-lang.org/

      parent reply index

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <redmine.issue-16335.20191108163402@ruby-lang.org>
2019-11-08 16:34 ` [ruby-core:95761] " colinb2r
2019-11-08 17:01 ` [ruby-core:95762] " wishdev
2019-11-08 23:13 ` merch-redmine [this message]

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.ruby-lang.org/en/community/mailing-lists/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=redmine.journal-82582.20191108231334.370c86d3aee171c9@ruby-lang.org \
    --to=ruby-core@ruby-lang.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

ruby-core@ruby-lang.org archive (unofficial mirror)

Archives are clonable: git clone --mirror https://public-inbox.org/ruby-core

Example config snippet for mirrors

Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.lang.ruby.core
	nntp://ou63pmih66umazou.onion/inbox.comp.lang.ruby.core
	nntp://news.gmane.org/gmane.comp.lang.ruby.core

 note: .onion URLs require Tor: https://www.torproject.org/

AGPL code for this site: git clone https://public-inbox.org/public-inbox.git