Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unimportant bug in JRuby? Date.civil and Date.jd raise exceptions when unusual - but should not be "invalid"? - Start Gregorian are used #5964

Open
colinb2r opened this issue Nov 8, 2019 · 1 comment

Comments

@colinb2r
Copy link

colinb2r commented Nov 8, 2019

I'm more than happy to try to answer any questions on my comments below and/or the attached program demonstrating what I think is a bug in JRuby.

Environment

  • jruby 9.2.8.0 (2.5.3) 2019-08-12 a1ac7ff Java HotSpot(TM) 64-Bit Server VM 25.231-b11 on 1.8.0_231-b11 +jit [mswin32-x86_64]
  • no command line flags
  • Lenovo Thinkpad T420 64bits running Microsoft Windows 7 & Java 1.8.0_231
  • standard JRuby Microsoft Windows installation

Expected Behavior

For Date.civil and Date.jd, C Ruby allows any finite value of Start Gregorian to be:
1582-01-01 jd 2298874 <= Start Gregorian <= 1931-01-13 jd 2426355
** See the attached program (which has the output from running on C Ruby and JRuby). **
jruby-date-start-gregorian-bug.rb.txt

Actual Behavior

By contrast JRuby allows - with a few exceptions - all finite values of Start Gregorian allowed, and many finite values before and after the allowed range for C Ruby.

The few exceptions seem to be when Start Gregorian = (y-03-01) - (1 day) + (skipped days),
where y is exactly divisible by 100 but not by 400, and
skipped days = (Julian y-03-01 - Gregorian y-03-01) = y / 100 - y.div(400) - 2 =
Date.civil(y,3,1, Date::JULIAN).jd - Date.civil(y,3,1, Date::GREGORIAN).jd

For example, using Date.civil(3333, 3, 3, sg):
SG 1700-03-10 jd 2342041 sk 11: #=> 3333-12-31 jd 2938778 sg 2342041 sk 23
SG 1700-03-11 jd 2342042 sk 11: #=> #<ArgumentError: invalid date>
SG 1700-03-12 jd 2342043 sk 11: #=> 3333-12-31 jd 2938778 sg 2342043 sk 23

  • I can't think of any reason why JRuby Date seems to be regarding these few Start Gregorian values as "invalid".

  • Assuming this is a bug, not a feature, it's an unimportant one: I think it extremely unlikely that any of these "invalid" Start Gregorian values will be used in practice.

  • But it might be indicative of a more important underlying problem with the JRuby implementation of Date.

I have looked in RubyDate.java to try to find why Date.civil and Date.jd raise exceptions when these apparently "invalid" Start Gregorian values are used,. But whilst I am very familiar with Julian and Gregorian calendar algorithms, and am reasonably familiar with their use in C Ruby "time_c" and "date_core.c", I admit to struggling to find in RubyDate.java the underlying code for implementing Date.civil and Date.jd: if someone could give me a few pointers to that, I might be able to work out what's happening.

** Aside:

  • I didn't have any problems running the attached program with C Ruby - no exceptions were raised, and it always ran very fast.

  • By contrast, running it with JRuby, exceptions were raised for those "invalid" Start Gregorian values, and it was relatively slow, taking about 50x longer than C Ruby, and that is excluding the Java start-up overhead.

Moreover, when running the program on JRuby searching for "invalid" Start Gregorian values over a very large range of years:

(i) Eventually the program temporarily paused for quite a long time, I think doing garbage collection.

(ii) On one occasion the program crashed with this error message:

Exception in thread "Thread-1" java.lang.OutOfMemoryError: GC overhead limit exceeded
at org.jruby.RubyThread.adoptThread(RubyThread.java:587)
at org.jruby.RubyThread.adopt(RubyThread.java:582)
at org.jruby.internal.runtime.ThreadService.adoptCurrentThread(ThreadService.java:218)
at org.jruby.internal.runtime.ThreadService.getCurrentContext(ThreadService.java:202)
at org.jruby.Ruby.getCurrentContext(Ruby.java:2766)
at org.jruby.Ruby.tearDown(Ruby.java:3318)
at org.jruby.Ruby.tearDown(Ruby.java:3305)
at org.jruby.Main$1.run(Main.java:285)

(iii) On another occasion this error message was shown:

Error: Your application used more memory than the automatic cap of 885MB.
Specify -J-Xmx####M to increase it (#### = cap size in MB).
Specify -w for full java.lang.OutOfMemoryError: Java heap space stack trace

  • This makes me wonder:
    (1) Is JRuby really sometimes taking much longer than C Ruby in making calculations with Date class?
    (2) If it is, maybe it's possible to speed up the Date class running in JRuby? Should Java be a lot slower than C for Date class calculations?
    (3) Is a possible reason for here JRuby Date seeming much slower than C Ruby Date that JRuby is creating a lot of objects but C Ruby is mainly working with integers and floating point?

I found this possible bug because I have been looking at ways of making C Ruby classes Time and Date faster, by in some functions using integer arithmetic instead of floating point double arithmetic, and also by sometimes using faster algorithms. For example, using this approach the C Ruby "date_core.c" function "c_valid_civil_p" (and functions it uses) can be rewritten to take about 30% of the time that the current version takes.

So there may be scope for making JRuby Date (and maybe also Time - I haven't yet tried benchmarking that against the C Ruby implementation) much faster?

@headius
Copy link
Member

headius commented Feb 16, 2020

The error might be due to Joda Time being a much stricter time library than whatever CRuby uses. We have had other cases where "weird" or actually "invalid" dates got rejected.

I suspect most of the performance issue is due to the exceptions being raised; exception stack traces are very expensive to generate on the JVM, and those errors appear to be happening as part of your benchmark. I would not expect to see JRuby's date handling be slower than CRuby if there's no errors being raised.

As for the error itself, it's raised in response to the GJChronology lookup here:

return GJChronology.getInstance(zone, cutover);

And the exception from Joda Time looks like this:

org.joda.time.IllegalFieldValueException: Value 29 for dayOfMonth must be in the range [1,28]
	at org.jruby.dist/org.joda.time.field.FieldUtils.verifyValueBounds(FieldUtils.java:277)
	at org.jruby.dist/org.joda.time.chrono.BasicChronology.getDateMidnightMillis(BasicChronology.java:632)
	at org.jruby.dist/org.joda.time.chrono.BasicChronology.getDateTimeMillis0(BasicChronology.java:186)
	at org.jruby.dist/org.joda.time.chrono.BasicChronology.getDateTimeMillis(BasicChronology.java:160)
	at org.jruby.dist/org.joda.time.chrono.GregorianChronology.getDateTimeMillis(GregorianChronology.java:44)
	at org.jruby.dist/org.joda.time.chrono.GJChronology.convertByYear(GJChronology.java:83)
	at org.jruby.dist/org.joda.time.chrono.GJChronology.julianToGregorianByYear(GJChronology.java:577)
	at org.jruby.dist/org.joda.time.chrono.GJChronology.assemble(GJChronology.java:486)
	at org.jruby.dist/org.joda.time.chrono.AssembledChronology.setFields(AssembledChronology.java:323)
	at org.jruby.dist/org.joda.time.chrono.AssembledChronology.<init>(AssembledChronology.java:102)
	at org.jruby.dist/org.joda.time.chrono.GJChronology.<init>(GJChronology.java:262)
	at org.jruby.dist/org.joda.time.chrono.GJChronology.getInstance(GJChronology.java:206)
	at org.jruby.dist/org.joda.time.chrono.GJChronology.getInstance(GJChronology.java:172)
	at org.jruby.dist/org.jruby.ext.date.RubyDate.getChronology(RubyDate.java:1528)
	at org.jruby.dist/org.jruby.ext.date.RubyDate.getChronology(RubyDate.java:1518)
	at org.jruby.dist/org.jruby.ext.date.RubyDate.civilImpl(RubyDate.java:417)
	at org.jruby.dist/org.jruby.ext.date.RubyDate.civil(RubyDate.java:439)
	at org.jruby.dist/org.jruby.ext.date.RubyDate$INVOKER$s$civil.call(RubyDate$INVOKER$s$civil.gen)
	at org.jruby.dist/org.jruby.runtime.callsite.CachingCallSite.call(CachingCallSite.java:74)
	at jruby_minus_date_minus_start_minus_gregorian_minus_bug.invokeOther31:civil(jruby-date-start-gregorian-bug.rb:32)
	at jruby_minus_date_minus_start_minus_gregorian_minus_bug.RUBY$method$qcivil$3(jruby-date-start-gregorian-bug.rb:32)
	at org.jruby.dist/org.jruby.internal.runtime.methods.CompiledIRMethod.call(CompiledIRMethod.java:184)
	at org.jruby.dist/org.jruby.runtime.callsite.CachingCallSite.cacheAndCall(CachingCallSite.java:396)
	at org.jruby.dist/org.jruby.runtime.callsite.CachingCallSite.call(CachingCallSite.java:205)
	at jruby_minus_date_minus_start_minus_gregorian_minus_bug.invokeOther55:qcivil(jruby-date-start-gregorian-bug.rb:70)
	at jruby_minus_date_minus_start_minus_gregorian_minus_bug.RUBY$block$qqq$6(jruby-date-start-gregorian-bug.rb:70)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants