[ 
https://issues.apache.org/jira/browse/LUCENE-7966?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16170377#comment-16170377
 ] 

Uwe Schindler edited comment on LUCENE-7966 at 9/18/17 5:52 PM:
----------------------------------------------------------------

I implemented by latest idea based again on Robert's patch: 
https://github.com/apache/lucene-solr/compare/master...uschindler:jira/LUCENE-7966-v2

This approach is much more clean: We compile against Robert's replacement 
classes {{FutureObjects}} and {{FutureArrays}} (that have to contain the same 
method signatures as the Java 9 original, but we can add a test for this later 
with smoketester) as usual with Java 8. Before packaging the JAR file we read 
all class files and patch all {{FutureObjects/FutureArrays}} references to 
refer to the Java 9 class. The patched output is sent to a separate folder 
{{build/classes/java9}}. The JAR file is then packaged to take both variants, 
placing the patched ones in the Java 9 MultiRelease part.

Currently only the lucene-core.jar file uses the patched stuff, so stuff 
outside lucene-core (e.g., codecs) does not yet automatically add Java 9 
variants, instead it will use Robert's classes. If this is the way to go, I 
will move the patcher to the global tools directory and we can apply patching 
to all JAR files of the distribution. WARNING: We cannot support Maven builds 
here, Maven always builds a Java8-only JAR file!

[~mikemccand], [~jpountz]: Could you build a lucene-core.jar file with the 
above branch on Github and do your tests again? The main difference here is 
that the JAR file no longer contains a delegator class. Instead all class files 
that were originally compiled with FutureObjects/FutureArrays (for Java 8 
support) are patched to directly use the Java 9 Arrays/Objects methods, without 
using a delegator class. Keep in mind: This currently only support 
lucene-core.jar, the codecs JAR file is not yet Multirelease with this patch.

When building with {{ant jar}} inside {{lucene/core}} you should see output 
like this:
{noformat}
     [compile shit...................]
     [copy] Copying 3 files to C:\Users\Uwe 
Schindler\Projects\lucene\trunk-lusolr1\lucene\build\core\classes\java

-mrjar-classes-uptodate:

resolve-groovy:
[ivy:cachepath] :: resolving dependencies :: 
org.codehaus.groovy#groovy-all-caller;working
[ivy:cachepath]         confs: [default]
[ivy:cachepath]         found org.codehaus.groovy#groovy-all;2.4.8 in public
[ivy:cachepath] :: resolution report :: resolve 170ms :: artifacts dl 5ms
        ---------------------------------------------------------------------
        |                  |            modules            ||   artifacts   |
        |       conf       | number| search|dwnlded|evicted|| number|dwnlded|
        ---------------------------------------------------------------------
        |      default     |   1   |   0   |   0   |   0   ||   1   |   0   |
        ---------------------------------------------------------------------

patch-mrjar-classes:
[ivy:cachepath] :: resolving dependencies :: 
org.ow2.asm#asm-commons-caller;working
[ivy:cachepath]         confs: [default]
[ivy:cachepath]         found org.ow2.asm#asm-commons;5.1 in public
[ivy:cachepath]         found org.ow2.asm#asm-tree;5.1 in public
[ivy:cachepath]         found org.ow2.asm#asm;5.1 in public
[ivy:cachepath] :: resolution report :: resolve 701ms :: artifacts dl 8ms
        ---------------------------------------------------------------------
        |                  |            modules            ||   artifacts   |
        |       conf       | number| search|dwnlded|evicted|| number|dwnlded|
        ---------------------------------------------------------------------
        |      default     |   3   |   0   |   0   |   0   ||   3   |   0   |
        ---------------------------------------------------------------------
   [groovy] Remapped: 
org/apache/lucene/analysis/tokenattributes/CharTermAttributeImpl
   [groovy] Remapped: org/apache/lucene/codecs/compressing/LZ4
   [groovy] Remapped: org/apache/lucene/document/BinaryPoint$2
   [groovy] Remapped: org/apache/lucene/document/DoubleRange
   [groovy] Remapped: org/apache/lucene/document/FloatRange
   [groovy] Remapped: org/apache/lucene/document/IntRange
   [groovy] Remapped: org/apache/lucene/document/LongRange
   [groovy] Remapped: org/apache/lucene/index/BitsSlice
   [groovy] Remapped: org/apache/lucene/index/CodecReader
   [groovy] Remapped: org/apache/lucene/index/MergeReaderWrapper
   [groovy] Remapped: org/apache/lucene/search/BooleanScorer$TailPriorityQueue
   [groovy] Remapped: org/apache/lucene/util/BytesRef
   [groovy] Remapped: org/apache/lucene/util/BytesRefArray
   [groovy] Remapped: 
org/apache/lucene/util/CharsRef$UTF16SortedAsUTF8Comparator
   [groovy] Remapped: org/apache/lucene/util/CharsRef
   [groovy] Remapped: org/apache/lucene/util/IntsRef
   [groovy] Remapped: org/apache/lucene/util/LongsRef
   [groovy] Remapped: org/apache/lucene/util/StringHelper
   [groovy] Remapped: org/apache/lucene/util/automaton/Automaton$Builder
   [groovy] Remapped: org/apache/lucene/util/automaton/Automaton
   [groovy] Remapped 20 class files for Java 9 to: C:\Users\Uwe 
Schindler\Projects\lucene\trunk-lusolr1\lucene\build\core\classes\java9
    [touch] Creating C:\Users\Uwe 
Schindler\Projects\lucene\trunk-lusolr1\lucene\build\core\patch-mrjar.stamp

jar-core:
      [jar] Building jar: C:\Users\Uwe 
Schindler\Projects\lucene\trunk-lusolr1\lucene\build\core\lucene-core-8.0.0-SNAPSHOT.jar

jar:

BUILD SUCCESSFUL
Total time: 31 seconds
{noformat}


was (Author: thetaphi):
I implemented by latest idea based again on Robert's patch: 
https://github.com/apache/lucene-solr/compare/master...uschindler:jira/LUCENE-7966-v2

This approach is much more clean: We compile against Robert's replacement 
classes {{FutureObjects}} and {{FutureArrays}} (that have to contain the same 
method signatures as the Java 9 original, but we can add a test for this later 
with smoketester) as usual with Java 8. Before packaging the JAR file we read 
all class files and patch all {{FutureObjects/FutureArrays}} references to 
refer to the Java 9 class. The patched output is sent to a separate folder 
{{build/classes/java9}}. The JAR file is then packaged to take both variants, 
placing the patched ones in the Java 9 MultiRelease part.

Currently only the lucene-core.jar file uses the patched stuff, so stuff 
outside lucene-core (e.g., codecs) does not yet automatically add Java 9 
variants, instead it will use Robert's classes. If this is the way to go, I 
will move the patcher to the global tools directory and we can apply patching 
to all JAR files of the distribution. WARNING: We cannot support Maven builds 
here, Maven always builds a Java8-only JAR file!

[~mikemccand], [~jpountz]: Could you build a lucene-core.jar file with the 
above branch on Github and do your tests again? The main difference here is 
that the JAR file no longer contains a delegator class. Instead all class files 
that were originally compiled with FutureObjects/FutureArrays (for Java 8 
support) are patched to directly use the Java 9 Arrays/Objects methods, without 
using a delegator class. Keep in mind: This currently only support 
lucene-core.jar, the codecs JAR file is not yet Multirelease with this patch.

When building with {{ant jar}} inside {{lucene/core}} you should see output 
like this:
{{noformat}}
     [compile shit...................]
     [copy] Copying 3 files to C:\Users\Uwe 
Schindler\Projects\lucene\trunk-lusolr1\lucene\build\core\classes\java

-mrjar-classes-uptodate:

resolve-groovy:
[ivy:cachepath] :: resolving dependencies :: 
org.codehaus.groovy#groovy-all-caller;working
[ivy:cachepath]         confs: [default]
[ivy:cachepath]         found org.codehaus.groovy#groovy-all;2.4.8 in public
[ivy:cachepath] :: resolution report :: resolve 170ms :: artifacts dl 5ms
        ---------------------------------------------------------------------
        |                  |            modules            ||   artifacts   |
        |       conf       | number| search|dwnlded|evicted|| number|dwnlded|
        ---------------------------------------------------------------------
        |      default     |   1   |   0   |   0   |   0   ||   1   |   0   |
        ---------------------------------------------------------------------

patch-mrjar-classes:
[ivy:cachepath] :: resolving dependencies :: 
org.ow2.asm#asm-commons-caller;working
[ivy:cachepath]         confs: [default]
[ivy:cachepath]         found org.ow2.asm#asm-commons;5.1 in public
[ivy:cachepath]         found org.ow2.asm#asm-tree;5.1 in public
[ivy:cachepath]         found org.ow2.asm#asm;5.1 in public
[ivy:cachepath] :: resolution report :: resolve 701ms :: artifacts dl 8ms
        ---------------------------------------------------------------------
        |                  |            modules            ||   artifacts   |
        |       conf       | number| search|dwnlded|evicted|| number|dwnlded|
        ---------------------------------------------------------------------
        |      default     |   3   |   0   |   0   |   0   ||   3   |   0   |
        ---------------------------------------------------------------------
   [groovy] Remapped: 
org/apache/lucene/analysis/tokenattributes/CharTermAttributeImpl
   [groovy] Remapped: org/apache/lucene/codecs/compressing/LZ4
   [groovy] Remapped: org/apache/lucene/document/BinaryPoint$2
   [groovy] Remapped: org/apache/lucene/document/DoubleRange
   [groovy] Remapped: org/apache/lucene/document/FloatRange
   [groovy] Remapped: org/apache/lucene/document/IntRange
   [groovy] Remapped: org/apache/lucene/document/LongRange
   [groovy] Remapped: org/apache/lucene/index/BitsSlice
   [groovy] Remapped: org/apache/lucene/index/CodecReader
   [groovy] Remapped: org/apache/lucene/index/MergeReaderWrapper
   [groovy] Remapped: org/apache/lucene/search/BooleanScorer$TailPriorityQueue
   [groovy] Remapped: org/apache/lucene/util/BytesRef
   [groovy] Remapped: org/apache/lucene/util/BytesRefArray
   [groovy] Remapped: 
org/apache/lucene/util/CharsRef$UTF16SortedAsUTF8Comparator
   [groovy] Remapped: org/apache/lucene/util/CharsRef
   [groovy] Remapped: org/apache/lucene/util/IntsRef
   [groovy] Remapped: org/apache/lucene/util/LongsRef
   [groovy] Remapped: org/apache/lucene/util/StringHelper
   [groovy] Remapped: org/apache/lucene/util/automaton/Automaton$Builder
   [groovy] Remapped: org/apache/lucene/util/automaton/Automaton
   [groovy] Remapped 20 class files for Java 9 to: C:\Users\Uwe 
Schindler\Projects\lucene\trunk-lusolr1\lucene\build\core\classes\java9
    [touch] Creating C:\Users\Uwe 
Schindler\Projects\lucene\trunk-lusolr1\lucene\build\core\patch-mrjar.stamp

jar-core:
      [jar] Building jar: C:\Users\Uwe 
Schindler\Projects\lucene\trunk-lusolr1\lucene\build\core\lucene-core-8.0.0-SNAPSHOT.jar

jar:

BUILD SUCCESSFUL
Total time: 31 seconds
{{noformat}}

> build mr-jar and use some java 9 methods if available
> -----------------------------------------------------
>
>                 Key: LUCENE-7966
>                 URL: https://issues.apache.org/jira/browse/LUCENE-7966
>             Project: Lucene - Core
>          Issue Type: Improvement
>          Components: general/build
>            Reporter: Robert Muir
>         Attachments: LUCENE-7966.patch, LUCENE-7966.patch, LUCENE-7966.patch, 
> LUCENE-7966.patch, LUCENE-7966.patch
>
>
> See background: http://openjdk.java.net/jeps/238
> It would be nice to use some of the newer array methods and range checking 
> methods in java 9 for example, without waiting for lucene 10 or something. If 
> we build an MR-jar, we can start migrating our code to use java 9 methods 
> right now, it will use optimized methods from java 9 when thats available, 
> otherwise fall back to java 8 code.  
> This patch adds:
> {code}
> Objects.checkIndex(int,int)
> Objects.checkFromToIndex(int,int,int)
> Objects.checkFromIndexSize(int,int,int)
> Arrays.mismatch(byte[],int,int,byte[],int,int)
> Arrays.compareUnsigned(byte[],int,int,byte[],int,int)
> Arrays.equal(byte[],int,int,byte[],int,int)
> // did not add char/int/long/short/etc but of course its possible if needed
> {code}
> It sets these up in {{org.apache.lucene.future}} as 1-1 mappings to java 
> methods. This way, we can simply directly replace call sites with java 9 
> methods when java 9 is a minimum. Simple 1-1 mappings mean also that we only 
> have to worry about testing that our java 8 fallback methods work.
> I found that many of the current byte array methods today are willy-nilly and 
> very lenient for example, passing invalid offsets at times and relying on 
> compare methods not throwing exceptions, etc. I fixed all the instances in 
> core/codecs but have not looked at the problems with AnalyzingSuggester. Also 
> SimpleText still uses a silly method in ArrayUtil in similar crazy way, have 
> not removed that one yet.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org

Reply via email to