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

moving Ruby Java extensions into native #3802

Merged
merged 28 commits into from Apr 27, 2016

Conversation

Projects
None yet
4 participants
@kares
Member

kares commented Apr 14, 2016

basically all of the java/java_ext.rb pieces such as java.util.rb

makes JRuby perform better (up to 4x) with a simple java.util.ArrayList script ... will probably improve further after more warmup.

most of the time we do not need the full dynamic nature when extending behaviour of Java types e.g. java.util.List#sort using a Comparator implemented in Ruby (lot of "unnecessary" Java<->Ruby conversion) on each compareTo improves #1401 performance **~ 2x**

'old' .rb parts are loadable (without harm) and will be used for yard doc generation.

likely users do not use (know of) the provided Ruby extensions (which might change as we provide JI doc) much since I've run into several issues with non-working bits (e.g. java.util.regex.Matcher methods failing) or half-baked methods (e.g. java.util.List's [] and[]=compared toArray`'s) - most of them are actually easier to manage/reason in Java. also most of the extensions are now covered with specs.

there's also some new features along the way such as dup now working for Java arrays and collection types (on 1.7/9.0 collection.dup silently returns the same instance), dig for lists, uniformly returning Enumerator on each without block and more mentioned bellow.

further all collision methods (e.g. Java 8's sort) now have a ruby_xxx name as well so that users can easily "force" Ruby functionality on types e.g. java.util.ArrayList.class_eval { alias sort ruby_sort }.

CHANGELOG

  • unified java.lang.Comparable#compareTo with incompatible types now raising TypeError
  • java.util.Iterable#each (also java.util.Collection#each) returns Enumerator when no block given
  • java.util.Iterable#each_with_index returns Enumerator when no block given
  • java.util.List#rindex now using listIterator to loop backwards instead of get(index)
  • java.util.List#sort dup-s - returns the same type as target (previously returning ArrayList)
    NOTE: as previously only works on Java 7 as there's a sort collision with Java 8
  • java.util.List [] extension works as with Array (no index out of bounds errors)
  • java.util.List []= extension behaves like with an Array (grows list as needed)
    removal should be more effective across different kinds of list implementations
  • Ruby's dup and clone methods are expected to be working for most Collection types
    (except internal impl classes such as ones returned from java.util.Collections factories)
  • Java arrays now correctly dup
  • added java.util.regex.Pattern#casefold?
  • java.util.regex.Matcher is expected to quack very much like a Ruby MatchData
  • java.util.regex.Matcher supports named groups with begin/end (available on Java 8+)
  • added abstract? helper for java.lang.Class and java.lang.reflect.Method
  • new java.lang.Class#anonymous? helper
  • JRuby now provides ruby_xxx names for potentially conflicting methods e.g. java.util.List#sort on Java 8
    java.util.ArrayList.class_eval { alias sort ruby_sort } will make all array lists sort the Ruby way while the instance is being used in Ruby scripts.
@kares

This comment has been minimized.

Show comment
Hide comment
@kares

kares Apr 20, 2016

Member

there's also some more work on top of this where Ruby <-> Java collision methods have a ruby_xxx name

Member

kares commented Apr 20, 2016

there's also some more work on top of this where Ruby <-> Java collision methods have a ruby_xxx name

@enebo

This comment has been minimized.

Show comment
Hide comment
@enebo

enebo Apr 20, 2016

Member

@kares I have two comments on this PR.

  1. Do we really want to go from Ruby -> Java. I am ok with this personally but it feels like we are moving the wrong direction here at some level. At least I could see some people saying that. Performance is performance though...I guess we can always go back to ruby impl later again.

  2. You left .rb files around that you converted which is great since people may require them (In fact I would lay money on that). If so, then won't these Ruby impl's replace the Java ones? Is that ok?

Member

enebo commented Apr 20, 2016

@kares I have two comments on this PR.

  1. Do we really want to go from Ruby -> Java. I am ok with this personally but it feels like we are moving the wrong direction here at some level. At least I could see some people saying that. Performance is performance though...I guess we can always go back to ruby impl later again.

  2. You left .rb files around that you converted which is great since people may require them (In fact I would lay money on that). If so, then won't these Ruby impl's replace the Java ones? Is that ok?

@kares

This comment has been minimized.

Show comment
Hide comment
@kares

kares Apr 20, 2016

Member
  1. wasn't sure either, first just tried some parts and compared performance. than I felt like its worth doing as some of the non one liners were ugly to reason and also had 2-5x worst performance to start with. than it felt like a nightmare to have it half-half. feel like users will appreciate the speed even it these are non-native esp. since these are some of the most common methods and JRuby already had some of these in native parts e.g. java.util.Map

I think if we provide some yard docs (using the old .rb parts) for these it should sell well :)

also more refactoring will be easier and it also opens up a clean path for potential lazy ext (when the Java proxy class gets first used in Ruby-land) loading if at some point we want to.

  1. that makes sense - will add if false so that it doesnt do harm.
Member

kares commented Apr 20, 2016

  1. wasn't sure either, first just tried some parts and compared performance. than I felt like its worth doing as some of the non one liners were ugly to reason and also had 2-5x worst performance to start with. than it felt like a nightmare to have it half-half. feel like users will appreciate the speed even it these are non-native esp. since these are some of the most common methods and JRuby already had some of these in native parts e.g. java.util.Map

I think if we provide some yard docs (using the old .rb parts) for these it should sell well :)

also more refactoring will be easier and it also opens up a clean path for potential lazy ext (when the Java proxy class gets first used in Ruby-land) loading if at some point we want to.

  1. that makes sense - will add if false so that it doesnt do harm.
@kares

This comment has been minimized.

Show comment
Hide comment
@kares

kares Apr 20, 2016

Member

added commits dealing with Ruby vs Java naming collisions + the old .rb parts are loadable without harm.

Member

kares commented Apr 20, 2016

added commits dealing with Ruby vs Java naming collisions + the old .rb parts are loadable without harm.

@kares

This comment has been minimized.

Show comment
Hide comment
@kares

kares Apr 22, 2016

Member

... and JI doc is coming along nicely: http://kares.org/jruby-ji-doc/Java/java/util/List.html ... still things to do.

Member

kares commented Apr 22, 2016

... and JI doc is coming along nicely: http://kares.org/jruby-ji-doc/Java/java/util/List.html ... still things to do.

kares added some commits Apr 22, 2016

[ji] unify Java Comparable behaviour - make type incompatibility `Typ…
…eError`s

previously it was sometimes ClassCastException, TypeError (attempted coercion)
[ji] moving (and improving) **java.util.rb** extensions to native code
... and not just for speed (most List extensions also missed specs) :

- List [] and []= were improved to properly work like with a Ruby Array
- detecting random-acess List for faster iteration
- backward List iteration on rindex - for non random access using listIterator

- Collection +/- will need more work as currently dup doesn't clone java objects
[ji] add some Java object dup/clone support (ab)using the java.lang.C…
…loneable contract

... will work with most Java collection types as they provide a public clone method
 this also makes java.util.Collection's (previous) +/- additions actually work nicely !
[ji] also support dup/clone-ing collections that do not provide a Jav…
…a clone method

... we can do so if we're able to allocate a new instance of the same class
[ji] correctly dup and clone Java arrays
... prev only clone worked but without Ruby's semantics for cloning singleton class
[ji] moving **java.util.regex.rb** into native for more fine grained …
…control

Java's Matcher now aligned with Ruby's MatchData (including named groups)
[ji] return same list type as target on java.util.List#sort
(Java 7 only due sort method collision on Java 8)
[ji] use ruby_xxx method naming convention for collision methods on j…
…ava.util.Map

users will be able to do use ruby versions with a simple alias on concrete types
e.g. `java.util.HashMap.class_eval { alias merge ruby_merge }`

resolves #1249
[ji] array like first/last on java.util.List with ruby_xxx name (to h…
…andle potential collision)

e.g. java.util.LinkedList provides getFirst/getLast thus previously behaviour was :

```
java.util.ArrayList.new.first # nil
java.util.LinkedList.new.first # throwing NoSuchElementException
# will now behave consistent in Ruby-land with : 
java.util.LinkedList.class_eval { alias last ruby_last; alias first ruby_first }
```
[ji] re-def Enumerable#first on Collection so that it can be aliased …
…when it conflicts

... e.g. this would happen on java.util.Deque due having a getFirst method prescribed
@kares

This comment has been minimized.

Show comment
Hide comment
@kares

kares Apr 25, 2016

Member

just out of interest I checked memory usage before and after, there's a small decrease ~12% (2MB) :

before ~ 17,5M :

sun.management.MemoryImpl@3dfc5fb8
Heap Memory Usage
init = 123731968(120832K) used = 17943952(17523K) committed = 105381888(102912K) max = 1760034816(1718784K)
Non-Heap Memory Usage
init = 2555904(2496K) used = 39972136(39035K) committed = 40435712(39488K) max = -1(-1K)

sun.management.MemoryImpl@3dfc5fb8
Heap Memory Usage
init = 123731968(120832K) used = 17903408(17483K) committed = 106430464(103936K) max = 1760034816(1718784K)
Non-Heap Memory Usage
init = 2555904(2496K) used = 39953920(39017K) committed = 40435712(39488K) max = -1(-1K)

sun.management.MemoryImpl@3dfc5fb8
Heap Memory Usage
init = 123731968(120832K) used = 17947384(17526K) committed = 105906176(103424K) max = 1760034816(1718784K)
Non-Heap Memory Usage
init = 2555904(2496K) used = 39940656(39004K) committed = 40435712(39488K) max = -1(-1K)

after ~ 15,5M :

sun.management.MemoryImpl@76b0bfab
Heap Memory Usage
init = 123731968(120832K) used = 15950240(15576K) committed = 105381888(102912K) max = 1760034816(1718784K)
Non-Heap Memory Usage
init = 2555904(2496K) used = 40509776(39560K) committed = 41058304(40096K) max = -1(-1K)

sun.management.MemoryImpl@76b0bfab
Heap Memory Usage
init = 123731968(120832K) used = 15281808(14923K) committed = 104857600(102400K) max = 1760034816(1718784K)
Non-Heap Memory Usage
init = 2555904(2496K) used = 40523264(39573K) committed = 41058304(40096K) max = -1(-1K)

sun.management.MemoryImpl@76b0bfab
Heap Memory Usage
init = 123731968(120832K) used = 16022968(15647K) committed = 105381888(102912K) max = 1760034816(1718784K)
Non-Heap Memory Usage
init = 2555904(2496K) used = 40511240(39561K) committed = 41123840(40160K) max = -1(-1K)

Member

kares commented Apr 25, 2016

just out of interest I checked memory usage before and after, there's a small decrease ~12% (2MB) :

before ~ 17,5M :

sun.management.MemoryImpl@3dfc5fb8
Heap Memory Usage
init = 123731968(120832K) used = 17943952(17523K) committed = 105381888(102912K) max = 1760034816(1718784K)
Non-Heap Memory Usage
init = 2555904(2496K) used = 39972136(39035K) committed = 40435712(39488K) max = -1(-1K)

sun.management.MemoryImpl@3dfc5fb8
Heap Memory Usage
init = 123731968(120832K) used = 17903408(17483K) committed = 106430464(103936K) max = 1760034816(1718784K)
Non-Heap Memory Usage
init = 2555904(2496K) used = 39953920(39017K) committed = 40435712(39488K) max = -1(-1K)

sun.management.MemoryImpl@3dfc5fb8
Heap Memory Usage
init = 123731968(120832K) used = 17947384(17526K) committed = 105906176(103424K) max = 1760034816(1718784K)
Non-Heap Memory Usage
init = 2555904(2496K) used = 39940656(39004K) committed = 40435712(39488K) max = -1(-1K)

after ~ 15,5M :

sun.management.MemoryImpl@76b0bfab
Heap Memory Usage
init = 123731968(120832K) used = 15950240(15576K) committed = 105381888(102912K) max = 1760034816(1718784K)
Non-Heap Memory Usage
init = 2555904(2496K) used = 40509776(39560K) committed = 41058304(40096K) max = -1(-1K)

sun.management.MemoryImpl@76b0bfab
Heap Memory Usage
init = 123731968(120832K) used = 15281808(14923K) committed = 104857600(102400K) max = 1760034816(1718784K)
Non-Heap Memory Usage
init = 2555904(2496K) used = 40523264(39573K) committed = 41058304(40096K) max = -1(-1K)

sun.management.MemoryImpl@76b0bfab
Heap Memory Usage
init = 123731968(120832K) used = 16022968(15647K) committed = 105381888(102912K) max = 1760034816(1718784K)
Non-Heap Memory Usage
init = 2555904(2496K) used = 40511240(39561K) committed = 41123840(40160K) max = -1(-1K)

@headius

This comment has been minimized.

Show comment
Hide comment
@headius

headius Apr 27, 2016

Member

It's a lot to review but the move looks good, and it comes with a lot of edge case fixes and specs. 👍

Member

headius commented Apr 27, 2016

It's a lot to review but the move looks good, and it comes with a lot of edge case fixes and specs. 👍

@kares kares merged commit 08e4b1b into master Apr 27, 2016

0 of 3 checks passed

continuous-integration/appveyor/branch AppVeyor build failed
Details
continuous-integration/appveyor/pr AppVeyor build failed
Details
continuous-integration/travis-ci/pr The Travis CI build failed
Details

@kares kares deleted the ji-native branch Apr 27, 2016

@trejkaz

This comment has been minimized.

Show comment
Hide comment
@trejkaz

trejkaz Jul 21, 2017

False assumption? Guava's ImmutableList is Cloneable, but happens to return itself. See issue #4721.

False assumption? Guava's ImmutableList is Cloneable, but happens to return itself. See issue #4721.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment