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

Implements #614 #759

Closed
wants to merge 4 commits into from
Closed

Implements #614 #759

wants to merge 4 commits into from

Conversation

@influenza
Copy link
Contributor

@influenza influenza commented May 23, 2013

This pull request implements #614

I've performed the changes outlined in the issue description. Benchmarks show that the performance degradation is ~0.7%

influenza added 3 commits May 17, 2013
I've changed all of the relevant cases of anonymous KindOf classes to
use the new JavaClassKindOf. This cuts down on some of the noise
produced by anonymous inner classes, especially given that these
replacement points all had identical implementations.
@BanzaiMan
Copy link
Member

@BanzaiMan BanzaiMan commented Jun 8, 2013

We fixed CI. Could you rebase this PR off the current master? Thanks.

@influenza
Copy link
Contributor Author

@influenza influenza commented Jun 8, 2013

Hmm.. not building on the machine I've got currently.

[ERROR] /work/fun/jruby/maven/jruby-rake-plugin/src/main/java/org/jruby/maven/ClasspathMojo.java:[11,22] package org.jruby.embed does not exist

I'll be back on my dev machine Monday. Also, this PR still needs some additional benchmark lovin' before it is accepted.

@headius
Copy link
Member

@headius headius commented Jun 8, 2013

Removes 20 classes...very nice :-)

Let us know when you think it's ready to merge. 0.7% degradation of kind_of is not a concern.

@headius
Copy link
Member

@headius headius commented Jun 10, 2013

Actually I went ahead and pulled this to run some numbers. My tests aren't exhaustive, but they show some interesting results: https://gist.github.com/headius/5748268

Summary: On Java 6, success cases for kind_of? degrade slightly, but failure cases improve. On Java 7, success cases are the same or faster, and failure cases are considerably faster.

I'm going to run our test suites locally to ensure this is 100%, but otherwise I'll be merging it. If there's additional improvement possible, we can do it as separate PRs.

@headius
Copy link
Member

@headius headius commented Jun 10, 2013

Merged in bcc2674.

@headius headius closed this Jun 10, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants