Re: [PR] Use value-based LRU cache in NodeHash [lucene]

dungba88 commented on PR #12738: URL: https://github.com/apache/lucene/pull/12738#issuecomment-1793580112 Thank you @mikemccand ! Agree we should have a single changes entry summarizing all different PR -- This is an automated message from the Apache Git Service. To respond to the message

Re: [PR] Use value-based LRU cache in NodeHash [lucene]

mikemccand commented on PR #12738: URL: https://github.com/apache/lucene/pull/12738#issuecomment-1793476420 I merged to main, thank you @dungba88 for the fast iterations! I could barely keep up just reviewing :) After all this FST dust settles let's remember to add your CHANGES.txt e

Re: [PR] Use value-based LRU cache in NodeHash [lucene]

mikemccand merged PR #12738: URL: https://github.com/apache/lucene/pull/12738 -- 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-unsubscr...@lucene.

Re: [PR] Use value-based LRU cache in NodeHash [lucene]

dungba88 commented on code in PR #12738: URL: https://github.com/apache/lucene/pull/12738#discussion_r1382384038 ## lucene/core/src/java/org/apache/lucene/util/ByteBlockPool.java: ## @@ -38,6 +38,8 @@ public final class ByteBlockPool implements Accountable { /** Abstract cl

Re: [PR] Use value-based LRU cache in NodeHash [lucene]

mikemccand commented on code in PR #12738: URL: https://github.com/apache/lucene/pull/12738#discussion_r1382379237 ## lucene/core/src/test/org/apache/lucene/util/TestByteBlockPool.java: ## @@ -91,6 +92,10 @@ public void testLargeRandomBlocks() throws IOException { random(

Re: [PR] Use value-based LRU cache in NodeHash [lucene]

mikemccand commented on code in PR #12738: URL: https://github.com/apache/lucene/pull/12738#discussion_r1382379017 ## lucene/core/src/java/org/apache/lucene/util/ByteBlockPool.java: ## @@ -38,6 +38,8 @@ public final class ByteBlockPool implements Accountable { /** Abstract

Re: [PR] Use value-based LRU cache in NodeHash [lucene]

mikemccand commented on code in PR #12738: URL: https://github.com/apache/lucene/pull/12738#discussion_r1382378735 ## lucene/core/src/java/org/apache/lucene/util/fst/ByteBlockPoolReverseBytesReader.java: ## @@ -0,0 +1,69 @@ +/* + * Licensed to the Apache Software Foundation (ASF

Re: [PR] Use value-based LRU cache in NodeHash [lucene]

dungba88 commented on code in PR #12738: URL: https://github.com/apache/lucene/pull/12738#discussion_r1382304599 ## lucene/core/src/java/org/apache/lucene/util/fst/NodeHash.java: ## @@ -186,149 +218,228 @@ private long hash(FSTCompiler.UnCompiledNode node) { return h; }

Re: [PR] Use value-based LRU cache in NodeHash [lucene]

mikemccand commented on code in PR #12738: URL: https://github.com/apache/lucene/pull/12738#discussion_r1381812079 ## lucene/core/src/java/org/apache/lucene/util/fst/NodeHash.java: ## @@ -186,149 +218,228 @@ private long hash(FSTCompiler.UnCompiledNode node) { return h;

Re: [PR] Use value-based LRU cache in NodeHash [lucene]

mikemccand commented on code in PR #12738: URL: https://github.com/apache/lucene/pull/12738#discussion_r1381811085 ## lucene/core/src/java/org/apache/lucene/util/fst/NodeHash.java: ## @@ -186,149 +218,228 @@ private long hash(FSTCompiler.UnCompiledNode node) { return h;

Re: [PR] Use value-based LRU cache in NodeHash [lucene]

jpountz commented on code in PR #12738: URL: https://github.com/apache/lucene/pull/12738#discussion_r1381802489 ## lucene/core/src/java/org/apache/lucene/util/fst/NodeHash.java: ## @@ -186,149 +218,228 @@ private long hash(FSTCompiler.UnCompiledNode node) { return h; }

Re: [PR] Use value-based LRU cache in NodeHash [lucene]

mikemccand commented on PR #12738: URL: https://github.com/apache/lucene/pull/12738#issuecomment-1792327869 `Test2BFST` is happy, yay! ``` BUILD SUCCESSFUL in 56m 36s ``` -- This is an automated message from the Apache Git Service. To respond to the message, please log on to G

Re: [PR] Use value-based LRU cache in NodeHash [lucene]

mikemccand commented on code in PR #12738: URL: https://github.com/apache/lucene/pull/12738#discussion_r1381570983 ## lucene/core/src/java/org/apache/lucene/util/fst/NodeHash.java: ## @@ -186,149 +218,228 @@ private long hash(FSTCompiler.UnCompiledNode node) { return h;

Re: [PR] Use value-based LRU cache in NodeHash [lucene]

dungba88 commented on code in PR #12738: URL: https://github.com/apache/lucene/pull/12738#discussion_r1381565556 ## lucene/core/src/java/org/apache/lucene/util/fst/NodeHash.java: ## @@ -186,149 +218,228 @@ private long hash(FSTCompiler.UnCompiledNode node) { return h; }

Re: [PR] Use value-based LRU cache in NodeHash [lucene]

mikemccand commented on code in PR #12738: URL: https://github.com/apache/lucene/pull/12738#discussion_r1381564163 ## lucene/core/src/java/org/apache/lucene/util/fst/NodeHash.java: ## @@ -110,25 +117,39 @@ public long add(FSTCompiler.UnCompiledNode nodeIn) throws IOException {

Re: [PR] Use value-based LRU cache in NodeHash [lucene]

dungba88 commented on code in PR #12738: URL: https://github.com/apache/lucene/pull/12738#discussion_r1381559347 ## lucene/core/src/java/org/apache/lucene/util/fst/NodeHash.java: ## @@ -186,149 +218,228 @@ private long hash(FSTCompiler.UnCompiledNode node) { return h; }

Re: [PR] Use value-based LRU cache in NodeHash [lucene]

mikemccand commented on PR #12738: URL: https://github.com/apache/lucene/pull/12738#issuecomment-1792293648 Thanks @dungba88! I confirmed that `IndexToFST` now works again, and, when given "up to" `inf` RAM to use, it produces the same sized minimal `fst.bin` as main at `367244208 by

Re: [PR] Use value-based LRU cache in NodeHash [lucene]

mikemccand commented on code in PR #12738: URL: https://github.com/apache/lucene/pull/12738#discussion_r1381538494 ## lucene/core/src/java/org/apache/lucene/util/fst/ByteBlockPoolReverseBytesReader.java: ## @@ -0,0 +1,69 @@ +/* + * Licensed to the Apache Software Foundation (ASF

Re: [PR] Use value-based LRU cache in NodeHash [lucene]

mikemccand commented on code in PR #12738: URL: https://github.com/apache/lucene/pull/12738#discussion_r1381520513 ## lucene/core/src/java/org/apache/lucene/util/fst/NodeHash.java: ## @@ -328,7 +298,100 @@ private void rehash(long lastNodeAddress) throws IOException { }

Re: [PR] Use value-based LRU cache in NodeHash [lucene]

dungba88 commented on code in PR #12738: URL: https://github.com/apache/lucene/pull/12738#discussion_r1380008658 ## lucene/core/src/java/org/apache/lucene/util/fst/NodeHash.java: ## @@ -328,7 +298,100 @@ private void rehash(long lastNodeAddress) throws IOException { }

Re: [PR] Use value-based LRU cache in NodeHash [lucene]

dungba88 commented on code in PR #12738: URL: https://github.com/apache/lucene/pull/12738#discussion_r1380008658 ## lucene/core/src/java/org/apache/lucene/util/fst/NodeHash.java: ## @@ -328,7 +298,100 @@ private void rehash(long lastNodeAddress) throws IOException { }

Re: [PR] Use value-based LRU cache in NodeHash [lucene]

dungba88 commented on code in PR #12738: URL: https://github.com/apache/lucene/pull/12738#discussion_r1380008658 ## lucene/core/src/java/org/apache/lucene/util/fst/NodeHash.java: ## @@ -328,7 +298,100 @@ private void rehash(long lastNodeAddress) throws IOException { }

Re: [PR] Use value-based LRU cache in NodeHash [lucene]

dungba88 commented on code in PR #12738: URL: https://github.com/apache/lucene/pull/12738#discussion_r1380008658 ## lucene/core/src/java/org/apache/lucene/util/fst/NodeHash.java: ## @@ -328,7 +298,100 @@ private void rehash(long lastNodeAddress) throws IOException { }

Re: [PR] Use value-based LRU cache in NodeHash [lucene]

dungba88 commented on code in PR #12738: URL: https://github.com/apache/lucene/pull/12738#discussion_r1380008658 ## lucene/core/src/java/org/apache/lucene/util/fst/NodeHash.java: ## @@ -328,7 +298,100 @@ private void rehash(long lastNodeAddress) throws IOException { }

Re: [PR] Use value-based LRU cache in NodeHash [lucene]

dungba88 commented on code in PR #12738: URL: https://github.com/apache/lucene/pull/12738#discussion_r1380008658 ## lucene/core/src/java/org/apache/lucene/util/fst/NodeHash.java: ## @@ -328,7 +298,100 @@ private void rehash(long lastNodeAddress) throws IOException { }

Re: [PR] Use value-based LRU cache in NodeHash [lucene]

dungba88 commented on code in PR #12738: URL: https://github.com/apache/lucene/pull/12738#discussion_r1380001349 ## lucene/core/src/java/org/apache/lucene/util/fst/NodeHash.java: ## @@ -328,7 +298,100 @@ private void rehash(long lastNodeAddress) throws IOException { }

Re: [PR] Use value-based LRU cache in NodeHash [lucene]

dungba88 commented on code in PR #12738: URL: https://github.com/apache/lucene/pull/12738#discussion_r1380001349 ## lucene/core/src/java/org/apache/lucene/util/fst/NodeHash.java: ## @@ -328,7 +298,100 @@ private void rehash(long lastNodeAddress) throws IOException { }

Re: [PR] Use value-based LRU cache in NodeHash [lucene]

dungba88 commented on code in PR #12738: URL: https://github.com/apache/lucene/pull/12738#discussion_r1379960873 ## lucene/core/src/java/org/apache/lucene/util/fst/NodeHash.java: ## @@ -328,7 +298,100 @@ private void rehash(long lastNodeAddress) throws IOException { }

Re: [PR] Use value-based LRU cache in NodeHash [lucene]

dungba88 commented on code in PR #12738: URL: https://github.com/apache/lucene/pull/12738#discussion_r1379938642 ## lucene/core/src/java/org/apache/lucene/util/fst/NodeHash.java: ## @@ -110,25 +117,39 @@ public long add(FSTCompiler.UnCompiledNode nodeIn) throws IOException {

Re: [PR] Use value-based LRU cache in NodeHash [lucene]

mikemccand commented on code in PR #12738: URL: https://github.com/apache/lucene/pull/12738#discussion_r1379769524 ## lucene/core/src/java/org/apache/lucene/util/fst/FSTCompiler.java: ## @@ -145,7 +145,7 @@ private FSTCompiler( if (suffixRAMLimitMB < 0) { throw new I

Re: [PR] Use value-based LRU cache in NodeHash [lucene]

dungba88 commented on PR #12738: URL: https://github.com/apache/lucene/pull/12738#issuecomment-1790354114 Yes, I just noticed that, and pushed out a fix. Seems like I was using the primary table pos instead of the fallback pos. And I added an assertion to catch it earlier. -- This

Re: [PR] Use value-based LRU cache in NodeHash [lucene]

mikemccand commented on PR #12738: URL: https://github.com/apache/lucene/pull/12738#issuecomment-1790333126 Thanks @dungba88 -- I will review! But first I tried running `IndexToFST` (recently born helper tool, now in luceneutil) on a `wikimediumall` index, creating the FST from all of

Re: [PR] Use value-based LRU cache in NodeHash [lucene]

dungba88 commented on code in PR #12738: URL: https://github.com/apache/lucene/pull/12738#discussion_r1378882326 ## lucene/core/src/java/org/apache/lucene/util/fst/NodeHash.java: ## @@ -186,132 +214,99 @@ private long hash(FSTCompiler.UnCompiledNode node) { return h; }

Re: [PR] Use value-based LRU cache in NodeHash [lucene]

dungba88 commented on code in PR #12738: URL: https://github.com/apache/lucene/pull/12738#discussion_r1378882326 ## lucene/core/src/java/org/apache/lucene/util/fst/NodeHash.java: ## @@ -186,132 +214,99 @@ private long hash(FSTCompiler.UnCompiledNode node) { return h; }

Re: [PR] Use value-based LRU cache in NodeHash [lucene]

dungba88 commented on code in PR #12738: URL: https://github.com/apache/lucene/pull/12738#discussion_r1378841828 ## lucene/core/src/java/org/apache/lucene/util/fst/NodeHash.java: ## @@ -328,7 +323,128 @@ private void rehash(long lastNodeAddress) throws IOException { }

Re: [PR] Use value-based LRU cache in NodeHash [lucene]

dungba88 commented on code in PR #12738: URL: https://github.com/apache/lucene/pull/12738#discussion_r1378848941 ## lucene/core/src/java/org/apache/lucene/util/fst/NodeHash.java: ## @@ -107,28 +121,43 @@ public long add(FSTCompiler.UnCompiledNode nodeIn) throws IOException {

Re: [PR] Use value-based LRU cache in NodeHash [lucene]

mikemccand commented on code in PR #12738: URL: https://github.com/apache/lucene/pull/12738#discussion_r1378809307 ## lucene/core/src/java/org/apache/lucene/util/fst/NodeHash.java: ## @@ -107,28 +121,43 @@ public long add(FSTCompiler.UnCompiledNode nodeIn) throws IOException {

Re: [PR] Use value-based LRU cache in NodeHash [lucene]

dungba88 commented on code in PR #12738: URL: https://github.com/apache/lucene/pull/12738#discussion_r1378704913 ## lucene/core/src/java/org/apache/lucene/util/fst/NodeHash.java: ## @@ -186,119 +197,85 @@ private long hash(FSTCompiler.UnCompiledNode node) { return h; }

Re: [PR] Use value-based LRU cache in NodeHash [lucene]

dungba88 commented on code in PR #12738: URL: https://github.com/apache/lucene/pull/12738#discussion_r1378703915 ## lucene/core/src/java/org/apache/lucene/util/fst/NodeHash.java: ## @@ -186,119 +197,85 @@ private long hash(FSTCompiler.UnCompiledNode node) { return h; }

Re: [PR] Use value-based LRU cache in NodeHash [lucene]

gf2121 commented on code in PR #12738: URL: https://github.com/apache/lucene/pull/12738#discussion_r1378670256 ## lucene/core/src/java/org/apache/lucene/util/fst/NodeHash.java: ## @@ -186,119 +193,85 @@ private long hash(FSTCompiler.UnCompiledNode node) { return h; } -

Re: [PR] Use value-based LRU cache in NodeHash [lucene]

mikemccand commented on code in PR #12738: URL: https://github.com/apache/lucene/pull/12738#discussion_r1378650643 ## lucene/core/src/java/org/apache/lucene/util/fst/NodeHash.java: ## @@ -186,119 +197,85 @@ private long hash(FSTCompiler.UnCompiledNode node) { return h; }

Re: [PR] Use value-based LRU cache in NodeHash [lucene]

dungba88 commented on PR #12738: URL: https://github.com/apache/lucene/pull/12738#issuecomment-1788758864 > Eventually it's moved to fallback, and, maybe it never gets promoted back (single copy), or, maybe it does (+1 copy) There is actually already one copy before this, which is whe

Re: [PR] Use value-based LRU cache in NodeHash [lucene]

mikemccand commented on PR #12738: URL: https://github.com/apache/lucene/pull/12738#issuecomment-1788744551 > But I ended up using a `List` where each item is a node instead of ByteBlockPool due to the following reasons: Hmm -- this is sizable added RAM overhead per entry. Added arra

Re: [PR] Use value-based LRU cache in NodeHash [lucene]

dungba88 commented on code in PR #12738: URL: https://github.com/apache/lucene/pull/12738#discussion_r1378601593 ## lucene/core/src/java/org/apache/lucene/util/fst/NodeHash.java: ## @@ -186,119 +193,85 @@ private long hash(FSTCompiler.UnCompiledNode node) { return h; }

Re: [PR] Use value-based LRU cache in NodeHash [lucene]

mikemccand commented on code in PR #12738: URL: https://github.com/apache/lucene/pull/12738#discussion_r1378599798 ## lucene/core/src/java/org/apache/lucene/util/fst/NodeHash.java: ## @@ -214,7 +222,13 @@ private long hash(long node) throws IOException { * Compares an unfroz

Re: [PR] Use value-based LRU cache in NodeHash [lucene]

dungba88 commented on code in PR #12738: URL: https://github.com/apache/lucene/pull/12738#discussion_r1378577965 ## lucene/core/src/java/org/apache/lucene/util/fst/NodeHash.java: ## @@ -110,25 +110,34 @@ public long add(FSTCompiler.UnCompiledNode nodeIn) throws IOException {

Re: [PR] Use value-based LRU cache in NodeHash [lucene]

gf2121 commented on code in PR #12738: URL: https://github.com/apache/lucene/pull/12738#discussion_r1378573421 ## lucene/core/src/java/org/apache/lucene/util/fst/NodeHash.java: ## @@ -186,119 +193,85 @@ private long hash(FSTCompiler.UnCompiledNode node) { return h; } -

Re: [PR] Use value-based LRU cache in NodeHash [lucene]

gf2121 commented on code in PR #12738: URL: https://github.com/apache/lucene/pull/12738#discussion_r1378571480 ## lucene/core/src/java/org/apache/lucene/util/fst/NodeHash.java: ## @@ -110,25 +110,34 @@ public long add(FSTCompiler.UnCompiledNode nodeIn) throws IOException {

Re: [PR] Use value-based LRU cache in NodeHash [lucene]

dungba88 commented on PR #12738: URL: https://github.com/apache/lucene/pull/12738#issuecomment-1788406285 Ok, it's ready for review. I'll add the CHANGES.txt entry once it's approved. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to G

Re: [PR] Use value-based LRU cache in NodeHash [lucene]

dungba88 commented on code in PR #12738: URL: https://github.com/apache/lucene/pull/12738#discussion_r1378381108 ## lucene/core/src/java/org/apache/lucene/util/fst/NodeHash.java: ## @@ -186,119 +194,85 @@ private long hash(FSTCompiler.UnCompiledNode node) { return h; }

Re: [PR] Use value-based LRU cache in NodeHash [lucene]

dungba88 commented on code in PR #12738: URL: https://github.com/apache/lucene/pull/12738#discussion_r1378285803 ## lucene/core/src/java/org/apache/lucene/util/fst/NodeHash.java: ## @@ -269,36 +283,58 @@ private boolean nodesEqual(FSTCompiler.UnCompiledNode node, long address)

Re: [PR] Use value-based LRU cache in NodeHash [lucene]

dungba88 commented on code in PR #12738: URL: https://github.com/apache/lucene/pull/12738#discussion_r1378285803 ## lucene/core/src/java/org/apache/lucene/util/fst/NodeHash.java: ## @@ -269,36 +283,58 @@ private boolean nodesEqual(FSTCompiler.UnCompiledNode node, long address)

Re: [PR] Use value-based LRU cache in NodeHash [lucene]

dungba88 commented on PR #12738: URL: https://github.com/apache/lucene/pull/12738#issuecomment-1788188467 I solved most of the nocommits (only 1 left). But I ended up using a `List` where each item is a node instead of ByteBlockPool due to the following reasons: - With ByteBlockPool we h

Re: [PR] Use value-based LRU cache in NodeHash [lucene]

dungba88 commented on code in PR #12738: URL: https://github.com/apache/lucene/pull/12738#discussion_r138426 ## lucene/core/src/java/org/apache/lucene/util/fst/NodeHash.java: ## @@ -214,7 +222,13 @@ private long hash(long node) throws IOException { * Compares an unfrozen

Re: [PR] Use value-based LRU cache in NodeHash [lucene]

mikemccand commented on code in PR #12738: URL: https://github.com/apache/lucene/pull/12738#discussion_r1377721562 ## lucene/core/src/java/org/apache/lucene/util/fst/NodeHash.java: ## @@ -269,36 +283,58 @@ private boolean nodesEqual(FSTCompiler.UnCompiledNode node, long address

Re: [PR] Use value-based LRU cache in NodeHash [lucene]

dweiss commented on code in PR #12738: URL: https://github.com/apache/lucene/pull/12738#discussion_r1377416721 ## lucene/core/src/java/org/apache/lucene/util/fst/ReverseBytesReader.java: ## @@ -17,7 +17,7 @@ package org.apache.lucene.util.fst; /** Reads in reverse from a sin

Re: [PR] Use value-based LRU cache in NodeHash [lucene]

dungba88 commented on code in PR #12738: URL: https://github.com/apache/lucene/pull/12738#discussion_r1377383960 ## lucene/core/src/java/org/apache/lucene/util/fst/ReverseBytesReader.java: ## @@ -17,7 +17,7 @@ package org.apache.lucene.util.fst; /** Reads in reverse from a s

Re: [PR] Use value-based LRU cache in NodeHash [lucene]

dungba88 commented on code in PR #12738: URL: https://github.com/apache/lucene/pull/12738#discussion_r1377364142 ## lucene/core/src/java/org/apache/lucene/util/fst/ReverseBytesReader.java: ## @@ -17,7 +17,7 @@ package org.apache.lucene.util.fst; /** Reads in reverse from a s

Re: [PR] Use value-based LRU cache in NodeHash [lucene]

dungba88 commented on code in PR #12738: URL: https://github.com/apache/lucene/pull/12738#discussion_r1377360860 ## lucene/core/src/java/org/apache/lucene/util/fst/NodeHash.java: ## @@ -269,36 +283,58 @@ private boolean nodesEqual(FSTCompiler.UnCompiledNode node, long address)

Re: [PR] Use value-based LRU cache in NodeHash [lucene]

dungba88 commented on code in PR #12738: URL: https://github.com/apache/lucene/pull/12738#discussion_r1377358613 ## lucene/core/src/java/org/apache/lucene/util/fst/NodeHash.java: ## @@ -269,36 +283,58 @@ private boolean nodesEqual(FSTCompiler.UnCompiledNode node, long address)

Re: [PR] Use value-based LRU cache in NodeHash [lucene]

dungba88 commented on code in PR #12738: URL: https://github.com/apache/lucene/pull/12738#discussion_r1377355083 ## lucene/core/src/java/org/apache/lucene/util/fst/NodeHash.java: ## @@ -269,36 +283,58 @@ private boolean nodesEqual(FSTCompiler.UnCompiledNode node, long address)

Re: [PR] Use value-based LRU cache in NodeHash [lucene]

dungba88 commented on code in PR #12738: URL: https://github.com/apache/lucene/pull/12738#discussion_r1377351934 ## lucene/core/src/java/org/apache/lucene/util/fst/FSTCompiler.java: ## @@ -749,7 +750,6 @@ public void add(IntsRef input, T output) throws IOException { // f

Re: [PR] Use value-based LRU cache in NodeHash [lucene]

dungba88 commented on code in PR #12738: URL: https://github.com/apache/lucene/pull/12738#discussion_r1377348991 ## lucene/core/src/java/org/apache/lucene/util/fst/FSTCompiler.java: ## @@ -461,9 +457,14 @@ long addNode(FSTCompiler.UnCompiledNode nodeIn) throws IOException {

Re: [PR] Use value-based LRU cache in NodeHash [lucene]

mikemccand commented on code in PR #12738: URL: https://github.com/apache/lucene/pull/12738#discussion_r1377320240 ## lucene/core/src/java/org/apache/lucene/util/fst/FSTCompiler.java: ## @@ -461,9 +457,14 @@ long addNode(FSTCompiler.UnCompiledNode nodeIn) throws IOException {

[PR] Use value-based LRU cache in NodeHash [lucene]

dungba88 opened a new pull request, #12738: URL: https://github.com/apache/lucene/pull/12738 ### Description Fix #12714 First attempt to introduce value-based LRU cache in NodeHash. There are some inefficiencies, but the functionalities work. -- This is an automated message