You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@lucene.apache.org by GitBox <gi...@apache.org> on 2021/09/30 08:47:28 UTC

[GitHub] [lucene] uschindler opened a new pull request #335: LUCENE-10137: Refactor DirectReader and remove heavy code duplication

uschindler opened a new pull request #335:
URL: https://github.com/apache/lucene/pull/335


   See https://issues.apache.org/jira/browse/LUCENE-10137


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene] rmuir commented on pull request #335: LUCENE-10137: Refactor DirectReader and remove heavy code duplication

Posted by GitBox <gi...@apache.org>.
rmuir commented on pull request #335:
URL: https://github.com/apache/lucene/pull/335#issuecomment-931194035


   where is the "heavy code duplication" ? You still have method to decode for each bit size. But now we made things more complicated for hotspot.
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene] uschindler edited a comment on pull request #335: LUCENE-10137: Refactor DirectReader and remove heavy code duplication

Posted by GitBox <gi...@apache.org>.
uschindler edited a comment on pull request #335:
URL: https://github.com/apache/lucene/pull/335#issuecomment-931255937


   Hi,
   I will close this PR because with JDK 11 it is problematic.
   
   I improved the whole thing a bit by changing LongValues to be an interface instead of an abstract class, which made the DirectReader look like this:
   
   https://github.com/uschindler/lucene/blob/18e52b2b64bdc4020a32581935e0d44774aed39a/lucene/core/src/java/org/apache/lucene/util/packed/DirectReader.java
   
   As you see it is mush shorter and better readable, but for some reason it does not behave well yet. I will keep the other branch in my own repository for later investigation.
   
   The whole experiment is here: https://github.com/apache/lucene/compare/main...uschindler:jira/LUCENE-10137-v2


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene] uschindler commented on pull request #335: LUCENE-10137: Refactor DirectReader and remove heavy code duplication

Posted by GitBox <gi...@apache.org>.
uschindler commented on pull request #335:
URL: https://github.com/apache/lucene/pull/335#issuecomment-931098148


   Actually I will do a perf test in a minute, but the code and call behaviour here is more or less identical to before. Instead of creating the classes in source code we let the JDK do it for us.
   
   The only additional "overhead" is the get() method which calls the functional interface, but thats always inlined. It could be prevented when (as noted in the issue) we change LongValues to be a functional interface on its own. Currently it is an abstract class with one method, which is a desaster,


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene] uschindler closed pull request #335: LUCENE-10137: Refactor DirectReader and remove heavy code duplication

Posted by GitBox <gi...@apache.org>.
uschindler closed pull request #335:
URL: https://github.com/apache/lucene/pull/335


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene] uschindler commented on pull request #335: LUCENE-10137: Refactor DirectReader and remove heavy code duplication

Posted by GitBox <gi...@apache.org>.
uschindler commented on pull request #335:
URL: https://github.com/apache/lucene/pull/335#issuecomment-931258217


   FYI, with JDK 18 the other branch works fast, it is just Java 11 that cannot handle this.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene] uschindler commented on pull request #335: LUCENE-10137: Refactor DirectReader and remove heavy code duplication

Posted by GitBox <gi...@apache.org>.
uschindler commented on pull request #335:
URL: https://github.com/apache/lucene/pull/335#issuecomment-931145999


   Looks like there's a slowdown with facets. Digging (waiting for CPU profiles)...


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene] uschindler commented on pull request #335: LUCENE-10137: Refactor DirectReader and remove heavy code duplication

Posted by GitBox <gi...@apache.org>.
uschindler commented on pull request #335:
URL: https://github.com/apache/lucene/pull/335#issuecomment-931255937


   Hi,
   I will close this PR because with JDK 11 it is problematic.
   
   I improved the whole thing a bit by changing LongValues to be an interface instead of an abstract class, which made the DirectReader look like - which makes it much easier to read:
   
   https://github.com/uschindler/lucene/blob/18e52b2b64bdc4020a32581935e0d44774aed39a/lucene/core/src/java/org/apache/lucene/util/packed/DirectReader.java
   
   As you see it is mush shorter and better readable, but for some reason it does not behave well yet. I will keep the other branch in my own repository for later investigation.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene] jpountz commented on pull request #335: LUCENE-10137: Refactor DirectReader and remove heavy code duplication

Posted by GitBox <gi...@apache.org>.
jpountz commented on pull request #335:
URL: https://github.com/apache/lucene/pull/335#issuecomment-931093010


   I'm curious what luceneutil will report with this change, the sorting and faceting tasks should give us a good idea of whether there is a slowdown or not.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org