From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: AS4713 221.184.0.0/13 X-Spam-Status: No, score=-2.7 required=3.0 tests=BAYES_00,DKIM_ADSP_CUSTOM_MED, FORGED_GMAIL_RCVD,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,RCVD_IN_DNSWL_MED, SPF_HELO_NONE,SPF_PASS shortcircuit=no autolearn=no autolearn_force=no version=3.4.2 Received: from neon.ruby-lang.org (neon.ruby-lang.org [221.186.184.75]) by dcvr.yhbt.net (Postfix) with ESMTP id 2BB321F454 for ; Fri, 8 Nov 2019 17:01:54 +0000 (UTC) Received: from neon.ruby-lang.org (localhost [IPv6:::1]) by neon.ruby-lang.org (Postfix) with ESMTP id CFBA3120C03; Sat, 9 Nov 2019 02:01:42 +0900 (JST) Received: from o1678948x4.outbound-mail.sendgrid.net (o1678948x4.outbound-mail.sendgrid.net [167.89.48.4]) by neon.ruby-lang.org (Postfix) with ESMTPS id 1A11D120C02 for ; Sat, 9 Nov 2019 02:01:40 +0900 (JST) Received: by filter0080p3mdw1.sendgrid.net with SMTP id filter0080p3mdw1-12969-5DC59F7B-5C 2019-11-08 17:01:47.63921564 +0000 UTC m=+71496.281924838 Received: from herokuapp.com (unknown [18.209.44.199]) by ismtpd0055p1iad2.sendgrid.net (SG) with ESMTP id W4XSMlRERqGAlp-4bF40jA for ; Fri, 08 Nov 2019 17:01:47.507 +0000 (UTC) Date: Fri, 08 Nov 2019 17:01:47 +0000 (UTC) From: wishdev@gmail.com Message-ID: References: Mime-Version: 1.0 X-Redmine-MailingListIntegration-Message-Ids: 71387 X-Redmine-Project: ruby-trunk X-Redmine-Issue-Id: 16335 X-Redmine-Issue-Author: colin13rg X-Redmine-Sender: wishdev X-Mailer: Redmine X-Redmine-Host: bugs.ruby-lang.org X-Redmine-Site: Ruby Issue Tracking System X-Auto-Response-Suppress: All Auto-Submitted: auto-generated X-SG-EID: =?us-ascii?Q?Z4Ej7Bg37lK69JtZPFJ+UW67gqYKj9Iu8E0xhUBC8OWjOH2sK1DHxC9RFmfC4G?= =?us-ascii?Q?qjuuI7OA=2F8Lb7wTCsuUuOxZMuLVSPIaV2g129Iy?= =?us-ascii?Q?fl=2FANtwMGe2LJ1JvrYF8MwpkEzvaOK6aByqEfJm?= =?us-ascii?Q?EbK9LcdLP6uZgv=2FaE7u9LsqU37EImpKw=2FsUdZBH?= =?us-ascii?Q?EIzJyZeoV+swT=2F52eQOvq+QjEmM4GdvJ3RQ=3D=3D?= To: ruby-core@ruby-lang.org X-ML-Name: ruby-core X-Mail-Count: 95762 Subject: [ruby-core:95762] [Ruby master Feature#16335] C Ruby time.c leap_year_p and date_core.c c_gregorian_leap_p - two suggestions for minor changes X-BeenThere: ruby-core@ruby-lang.org X-Mailman-Version: 2.1.15 Precedence: list Reply-To: Ruby developers List-Id: Ruby developers List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: ruby-core-bounces@ruby-lang.org Sender: "ruby-core" Issue #16335 has been updated by wishdev (John Higgins). colin13rg (Colin Bartlett) wrote: ... > But if we rebracket the return statements as, for example: > return y % 4 == 0 && (y % 100 != 0 || y % 400 == 0); > Excellent analysis. > 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; > } > This is interesting because it squarely falls somewhere "notable" on the readability vs speed spectrum. The first suggestion above is clear to understand. This one not so much. Also I'm assuming a typo here in that the first if statement obviously should be if (y % 4 != 0) But excellent work! John ---------------------------------------- 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-82579 * 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/