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

Replace BigDecimal#!= by BigDecimal#nonzero?. #4199

Merged
merged 1 commit into from Oct 3, 2016

Conversation

felixvf
Copy link

@felixvf felixvf commented Oct 3, 2016

The comparison is very expensive, because it first tries to convert the operand (which is a constant Integer 0 here) into a BigDecimal. This in turn is very expensive (because the Integer is first converted into a String, and then, for operating on that String, a java.util.regex.Pattern object is compiled (in line https://github.com/felixvf/jruby/blob/cc79119315496ea019496affe7a6a79ebc1dbed9/core/src/main/java/org/jruby/ext/bigdecimal/RubyBigDecimal.java#L533)). Using #nonzero? should speed-up the loop considerably.

The comparison is very expensive, because it first tries to convert the operand (which is a constant Integer 0 here) into a BigDecimal. This in turn is very expensive (because the Integer is first converted into a String, and then, for operating on that String, a java.util.regex.Pattern object is compiled (in line https://github.com/felixvf/jruby/blob/cc79119315496ea019496affe7a6a79ebc1dbed9/core/src/main/java/org/jruby/ext/bigdecimal/RubyBigDecimal.java#L533)). Using #nonzero? should speed-up the loop considerably.
@felixvf felixvf force-pushed the fix_bigmath_log_performance branch from d7cd607 to 3acd172 Compare October 3, 2016 10:58
@kares kares added this to the JRuby 9.1.6.0 milestone Oct 3, 2016
@kares kares merged commit 8516d1f into jruby:master Oct 3, 2016
@kares
Copy link
Member

kares commented Oct 3, 2016

nice catch Felix, thanks

@felixvf felixvf deleted the fix_bigmath_log_performance branch October 3, 2016 12:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants