Copilot commented on code in PR #16858: URL: https://github.com/apache/lucene/pull/16858#discussion_r4316200297
########## lucene/benchmark/src/java/org/apache/lucene/benchmark/byTask/feeds/JoinDocMaker.java: ########## @@ -0,0 +1,226 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.lucene.benchmark.byTask.feeds; + +import java.io.IOException; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.SplittableRandom; +import java.util.concurrent.atomic.AtomicLong; +import java.util.function.LongUnaryOperator; +import org.apache.lucene.benchmark.byTask.utils.Config; +import org.apache.lucene.document.Document; +import org.apache.lucene.document.Field.Store; +import org.apache.lucene.document.LongPoint; +import org.apache.lucene.document.NumericDocValuesField; +import org.apache.lucene.document.SortedDocValuesField; +import org.apache.lucene.document.StringField; +import org.apache.lucene.util.BytesRef; + +/** + * Generates a products/skus 1:M dataset for {@link + * org.apache.lucene.benchmark.byTask.tasks.JoinSearchTask}: every sku refers to exactly one product + * by its primary key. Field values are a pure function of the document ordinal and its version (0 + * initially, bumped by {@link org.apache.lucene.benchmark.byTask.tasks.JoinUpdateTask}), so all + * layouts hold identical data. + * + * <p>Products carry the PK as indexed term {@link #ID} and as point {@link #ID_NUM}; skus carry + * their own {@link #ID} and the FK as {@link SortedDocValuesField} {@link #FK} and {@link + * NumericDocValuesField} {@link #FK_NUM}. Layouts, i.e. <b>join.layout</b>: + * + * <ul> + * <li><b>products</b>, <b>skus</b> - one side per index, for the term and numeric joins + * <li><b>mixed</b> - both sides as independent docs in one index; products carry their own id in + * {@link #FK}, as the global ordinals join requires + * <li><b>block</b> - each product's skus followed by the product, for the block join; every doc + * of a block carries the product id in {@link #PID}, to rewrite the block on update + * </ul> + * + * <p>Other config properties, see {@link Dataset}: <b>join.products</b> - Default: 1000; + * <b>join.skusPerProduct</b> - Default: 10; <b>join.seed</b> - Default: 42. + * + * <p>Index with {@code { AddJoinDocs } : *}; each call returns the next product group, and the + * {@link Dataset} goes into the commit user data. + */ +public class JoinDocMaker extends DocMaker { + + public static final String TYPE = "type"; + public static final String PRODUCT = "product"; + public static final String SKU = "sku"; + public static final String ID = "id"; + public static final String ID_NUM = "id_num"; + public static final String BRAND = "brand"; + public static final String FK = "fk"; + public static final String FK_NUM = "fk_num"; + public static final String PID = "pid"; + public static final String COLOR = "color"; + public static final String SIZE = "size"; + + public static final int BRANDS = 30; + public static final int COLORS = 20; + public static final int SIZES = 20; + + /** + * Size and seed of the generated data, which determine every document. Indexing records it in the + * commit user data, so updates regenerate documents consistently without repeating the config. + */ + public record Dataset(long products, int skusPerProduct, long seed) { + + private static final String PRODUCTS = "join.products"; + private static final String SKUS_PER_PRODUCT = "join.skusPerProduct"; + private static final String SEED = "join.seed"; + + public static Dataset fromConfig(Config config) { + return new Dataset( + config.get(PRODUCTS, 1000), config.get(SKUS_PER_PRODUCT, 10), config.get(SEED, 42)); + } + + /** Reads what {@link #toUserData()} wrote; fails if the index wasn't built by AddJoinDocs. */ + public static Dataset fromUserData(Iterable<Map.Entry<String, String>> userData) { + Map<String, String> data = new HashMap<>(); + userData.forEach(e -> data.put(e.getKey(), e.getValue())); + if (!data.containsKey(PRODUCTS)) { + throw new IllegalStateException("No join dataset in commit user data, see join-index.alg"); + } + return new Dataset( + Long.parseLong(data.get(PRODUCTS)), + Integer.parseInt(data.get(SKUS_PER_PRODUCT)), + Long.parseLong(data.get(SEED))); + } + + public Map<String, String> toUserData() { + return Map.of( + PRODUCTS, Long.toString(products), + SKUS_PER_PRODUCT, Integer.toString(skusPerProduct), + SEED, Long.toString(seed)); + } + + /** Product's skus followed by the product, i.e. the block join block. */ + public List<Document> block(long product, long version, LongUnaryOperator skuVersion) { + List<Document> block = skus("block", product, skuVersion); + block.add(product("block", product, version)); + return block; + } + + List<Document> skus(String layout, long product, LongUnaryOperator skuVersion) { + List<Document> skus = new ArrayList<>(skusPerProduct + 1); + for (long sku = product * skusPerProduct; sku < (product + 1) * skusPerProduct; sku++) { + skus.add(sku(layout, sku, skuVersion.applyAsLong(sku))); + } + return skus; + } + + public Document product(String layout, long product, long version) { + String id = productId(product); + Document doc = new Document(); + doc.add(new StringField(TYPE, PRODUCT, Store.NO)); + doc.add(new StringField(ID, id, Store.NO)); + doc.add(new LongPoint(ID_NUM, product)); + int brand = random(2 * product, version).nextInt(BRANDS); + doc.add(new StringField(BRAND, value(BRAND, brand), Store.NO)); + switch (layout) { + case "mixed" -> doc.add(new SortedDocValuesField(FK, new BytesRef(id))); + case "block" -> doc.add(new StringField(PID, id, Store.NO)); + default -> {} + } + return doc; + } + + public Document sku(String layout, long sku, long version) { + long product = sku / skusPerProduct; + String fk = productId(product); + SplittableRandom random = random(2 * sku + 1, version); + Document doc = new Document(); + doc.add(new StringField(TYPE, SKU, Store.NO)); + doc.add(new StringField(ID, skuId(sku), Store.NO)); + doc.add(new SortedDocValuesField(FK, new BytesRef(fk))); + doc.add(new NumericDocValuesField(FK_NUM, product)); + doc.add(new StringField(COLOR, value(COLOR, random.nextInt(COLORS)), Store.NO)); + doc.add(new StringField(SIZE, value(SIZE, random.nextInt(SIZES)), Store.NO)); + if (layout.equals("block")) { + doc.add(new StringField(PID, fk, Store.NO)); + } + return doc; + } + + private SplittableRandom random(long key, long version) { + return new SplittableRandom(seed + 31 * (31 * version + key)); + } + } + + private final AtomicLong next = new AtomicLong(); + private String layout; + private Dataset dataset; + + /** Keyword value of a generated attribute, e.g. {@code color7}. */ + public static String value(String field, int n) { + return field + n; + } + + public static String productId(long product) { + return "p" + product; + } + + public static String skuId(long sku) { + return "s" + sku; + } + + @Override + public void setConfig(Config config, ContentSource source) { + super.setConfig(config, source); + layout = config.get("join.layout", "mixed"); + dataset = Dataset.fromConfig(config); + if (!List.of("products", "skus", "mixed", "block").contains(layout)) { + throw new IllegalArgumentException("join.layout: " + layout); + } + } + + public Dataset getDataset() { + return dataset; + } + + @Override + public synchronized void resetInputs() throws IOException { + super.resetInputs(); + next.set(0); + } + + @Override + public Document makeDocument() { + throw new UnsupportedOperationException("use AddJoinDocs"); + } + + /** Next product group of {@code join.layout}, all at version 0. */ + public List<Document> makeGroup() throws NoMoreDataException { + long product = next.getAndIncrement(); + if (product >= dataset.products()) { + throw new NoMoreDataException(); + } + return switch (layout) { + case "products" -> List.of(dataset.product(layout, product, 0)); + case "skus" -> dataset.skus(layout, product, _ -> 0); + case "mixed" -> { + List<Document> group = dataset.skus(layout, product, _ -> 0); + group.addFirst(dataset.product(layout, product, 0)); Review Comment: `List<Document>` does not have `addFirst(...)` (this will not compile). Use an indexed insert (e.g., `group.add(0, ...)`) or build `group` as a `Deque`/`LinkedList` if you need `addFirst`. ########## lucene/benchmark/src/java/org/apache/lucene/benchmark/byTask/tasks/JoinSearchTask.java: ########## @@ -0,0 +1,172 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.lucene.benchmark.byTask.tasks; + +import static org.apache.lucene.benchmark.byTask.feeds.JoinDocMaker.BRAND; +import static org.apache.lucene.benchmark.byTask.feeds.JoinDocMaker.BRANDS; +import static org.apache.lucene.benchmark.byTask.feeds.JoinDocMaker.COLOR; +import static org.apache.lucene.benchmark.byTask.feeds.JoinDocMaker.COLORS; +import static org.apache.lucene.benchmark.byTask.feeds.JoinDocMaker.FK; +import static org.apache.lucene.benchmark.byTask.feeds.JoinDocMaker.FK_NUM; +import static org.apache.lucene.benchmark.byTask.feeds.JoinDocMaker.ID; +import static org.apache.lucene.benchmark.byTask.feeds.JoinDocMaker.ID_NUM; +import static org.apache.lucene.benchmark.byTask.feeds.JoinDocMaker.PRODUCT; +import static org.apache.lucene.benchmark.byTask.feeds.JoinDocMaker.SIZE; +import static org.apache.lucene.benchmark.byTask.feeds.JoinDocMaker.SIZES; +import static org.apache.lucene.benchmark.byTask.feeds.JoinDocMaker.TYPE; +import static org.apache.lucene.benchmark.byTask.feeds.JoinDocMaker.value; + +import java.util.ArrayList; +import java.util.List; +import java.util.Random; +import java.util.TreeSet; +import org.apache.lucene.benchmark.byTask.PerfRunData; +import org.apache.lucene.benchmark.byTask.feeds.JoinDocMaker; +import org.apache.lucene.index.SortedDocValues; +import org.apache.lucene.index.Term; +import org.apache.lucene.search.BooleanClause.Occur; +import org.apache.lucene.search.BooleanQuery; +import org.apache.lucene.search.IndexSearcher; +import org.apache.lucene.search.Query; +import org.apache.lucene.search.SearcherManager; +import org.apache.lucene.search.TermInSetQuery; +import org.apache.lucene.search.TermQuery; +import org.apache.lucene.search.join.BitSetProducer; +import org.apache.lucene.search.join.JoinUtil; +import org.apache.lucene.search.join.QueryBitSetProducer; +import org.apache.lucene.search.join.ScoreMode; +import org.apache.lucene.search.join.ToParentBlockJoinQuery; +import org.apache.lucene.util.BytesRef; + +/** + * Counts products joined from skus, over the indices written by {@link JoinDocMaker} (see + * conf/join-index.alg). Each query selects skus by a few colors or a few sizes, joins them to their + * products and filters products by a few brands. Every instance draws the same query sequence from + * <b>join.seed</b>, so instances with different algorithms are comparable. + * + * <p>The parameter is the algorithm: + * + * <ul> + * <li><b>term</b> - skus' {@link SortedDocValues} FK to products' PK terms (products, skus) + * <li><b>numeric</b> - skus' {@link org.apache.lucene.index.NumericDocValues} FK to products' PK + * points (products, skus) + * <li><b>globalOrd</b> - global ordinals over {@link SortedDocValues} FK (mixed) + * <li><b>block</b> - {@link ToParentBlockJoinQuery} (block) + * </ul> + * + * <p>Other config properties: <b>join.dir</b> and <b>join.searchThreads</b>, see {@link + * JoinIndices}; <b>join.scoreMode</b> - Default: None. + */ +public class JoinSearchTask extends PerfTask { + + private static final BitSetProducer PRODUCTS = + new QueryBitSetProducer(new TermQuery(new Term(TYPE, PRODUCT))); + + private final Random random; + private final ScoreMode scoreMode; + private String algorithm; + private SearcherManager fromManager; + private SearcherManager toManager; + private IndexSearcher from; + private IndexSearcher to; + private long totalHits; + + public JoinSearchTask(PerfRunData runData) { + super(runData); + random = new Random(runData.getConfig().get("join.seed", 42)); + scoreMode = ScoreMode.valueOf(runData.getConfig().get("join.scoreMode", "None")); + } + + @Override + public void setup() throws Exception { + super.setup(); + if (fromManager == null) { + JoinIndices indices = JoinIndices.get(getRunData()); + String layout = algorithm.equals("globalOrd") ? "mixed" : algorithm; + boolean split = algorithm.equals("term") || algorithm.equals("numeric"); + fromManager = indices.index(split ? "skus" : layout).manager(); + toManager = indices.index(split ? "products" : layout).manager(); + } + from = fromManager.acquire(); + to = toManager.acquire(); + } + + @Override + public int doLogic() throws Exception { + boolean byColor = random.nextBoolean(); + Query skus = terms(byColor ? COLOR : SIZE, byColor ? COLORS : SIZES, 2 + random.nextInt(3)); + Query brands = terms(BRAND, BRANDS, 1 + random.nextInt(3)); + Query join = + switch (algorithm) { + case "term" -> JoinUtil.createJoinQuery(FK, false, ID, skus, from, scoreMode); + case "numeric" -> + JoinUtil.createJoinQuery(FK_NUM, false, ID_NUM, Long.class, skus, from, scoreMode); + case "globalOrd" -> + JoinUtil.createJoinQuery( + FK, + skus, + new TermQuery(new Term(TYPE, PRODUCT)), + from, + scoreMode, + ((JoinIndices.JoinSearcher) from).ordinalMap()); + default -> new ToParentBlockJoinQuery(skus, PRODUCTS, scoreMode); Review Comment: The `default` switch branch implicitly treats any unexpected `algorithm` value as `block`. Since `block` is a valid explicit option, it’s clearer/safer to add a `case "block" -> ...` and have `default` throw (or otherwise fail fast) to prevent silent misuse if `algorithm` is ever unset or bypasses `setParams`. ########## lucene/benchmark/src/java/org/apache/lucene/benchmark/byTask/tasks/JoinUpdateTask.java: ########## @@ -0,0 +1,158 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.lucene.benchmark.byTask.tasks; + +import static org.apache.lucene.benchmark.byTask.feeds.JoinDocMaker.ID; +import static org.apache.lucene.benchmark.byTask.feeds.JoinDocMaker.PID; +import static org.apache.lucene.benchmark.byTask.feeds.JoinDocMaker.productId; +import static org.apache.lucene.benchmark.byTask.feeds.JoinDocMaker.skuId; + +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Random; +import org.apache.lucene.benchmark.byTask.PerfRunData; +import org.apache.lucene.benchmark.byTask.feeds.JoinDocMaker; +import org.apache.lucene.benchmark.byTask.feeds.JoinDocMaker.Dataset; +import org.apache.lucene.index.IndexWriter; +import org.apache.lucene.index.Term; + +/** + * Overrides a batch of random existing products and/or skus with new versions of their values (see + * {@link JoinDocMaker}), then refreshes the {@link JoinIndices} searchers, so it measures update + * plus NRT reopen. Data size and seed come from the {@link Dataset} recorded in the index at + * indexing time. + * + * <p>The parameter is the layout group: <b>split</b> (products and skus indices), <b>mixed</b> or + * <b>block</b>. The block layout rewrites the whole product block on any update, i.e. suffers + * update amplification. All groups draw the same update sequence from <b>join.seed</b>, so after + * the same number of updates all join algorithms agree. + * + * <p>Other config properties: <b>join.update.side</b> - products | skus | both. Default: both; + * <b>join.update.batch</b> - updates per task run. Default: 100. + */ +public class JoinUpdateTask extends PerfTask { + + /** Update sequence and the latest version of each updated doc, per layout group. */ + private static final class Versions { + final Random random; + final Map<Long, Long> products = new HashMap<>(); + final Map<Long, Long> skus = new HashMap<>(); + long version; + + Versions(long seed) { + random = new Random(seed); + } + } + + private final String side; + private final int batch; + private String group; + private JoinIndices.Index productIndex; + private JoinIndices.Index skuIndex; + private Versions versions; + private Dataset docs; + + public JoinUpdateTask(PerfRunData runData) { + super(runData); + side = runData.getConfig().get("join.update.side", "both"); + batch = runData.getConfig().get("join.update.batch", 100); + if (!List.of("products", "skus", "both").contains(side)) { + throw new IllegalArgumentException("join.update.side: " + side); + } + } + + @Override + public void setup() throws Exception { + super.setup(); + if (versions == null) { + PerfRunData runData = getRunData(); + JoinIndices indices = JoinIndices.get(runData); + productIndex = indices.index(group.equals("split") ? "products" : group); + skuIndex = indices.index(group.equals("split") ? "skus" : group); + docs = Dataset.fromUserData(productIndex.writer().getLiveCommitData()); + Dataset skus = Dataset.fromUserData(skuIndex.writer().getLiveCommitData()); + if (!docs.equals(skus)) { + throw new IllegalStateException("products " + docs + " but skus " + skus); + } + synchronized (runData) { + String key = JoinUpdateTask.class.getName() + "." + group; + versions = (Versions) runData.getPerfObject(key); + if (versions == null) { + versions = new Versions(runData.getConfig().get("join.seed", 42)); + runData.setPerfObject(key, versions); + } + } + } + } + + @Override + public int doLogic() throws Exception { + String productLayout = group.equals("split") ? "products" : group; + String skuLayout = group.equals("split") ? "skus" : group; + IndexWriter productWriter = productIndex.writer(); + IndexWriter skuWriter = skuIndex.writer(); + boolean products = !side.equals("skus"); + boolean skus = !side.equals("products"); + synchronized (versions) { + for (int i = 0; i < batch; i++) { + long product = versions.random.nextLong(docs.products()); + long sku = product * docs.skusPerProduct() + versions.random.nextInt(docs.skusPerProduct()); + long version = ++versions.version; + if (products) { + versions.products.put(product, version); + } + if (skus) { + versions.skus.put(sku, version); + } + if (group.equals("block")) { + productWriter.updateDocuments( + new Term(PID, productId(product)), + docs.block( + product, + versions.products.getOrDefault(product, 0L), + s -> versions.skus.getOrDefault(s, 0L))); + continue; + } + if (products) { + productWriter.updateDocument( + new Term(ID, productId(product)), docs.product(productLayout, product, version)); + } + if (skus) { + skuWriter.updateDocument(new Term(ID, skuId(sku)), docs.sku(skuLayout, sku, version)); + } + } + } + productIndex.manager().maybeRefreshBlocking(); + skuIndex.manager().maybeRefreshBlocking(); Review Comment: For `group` values where `productIndex` and `skuIndex` resolve to the same underlying index (e.g., `mixed` and `block`), this refresh is executed twice. Consider refreshing once when `productIndex == skuIndex` to avoid redundant work and reduce benchmark noise. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
