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/02/24 16:41:56 UTC

[GitHub] [lucene-solr] madrob commented on a change in pull request #2429: LUCENE-9791 Allow calling BytesRefHash#find concurrently

madrob commented on a change in pull request #2429:
URL: https://github.com/apache/lucene-solr/pull/2429#discussion_r582073943



##########
File path: lucene/core/src/test/org/apache/lucene/util/TestBytesRefHash.java
##########
@@ -267,6 +270,44 @@ public void testFind() throws Exception {
     }
   }
 
+  @Test
+  public void testConcurrentFind() throws Exception {
+    int num = atLeast(2);
+    for (int j = 0; j < num; j++) {
+      List<String> strings = new ArrayList<>();
+      for (int i = 0; i < 797; i++) {
+        final String str = TestUtil.randomRealisticUnicodeString(random(), 1, 1000);
+        hash.add(new BytesRef(str));
+        assertTrue(strings.add(str));
+      }
+
+      AtomicInteger miss = new AtomicInteger();
+      Thread[] threads = new Thread[10];
+      for (int i = 0; i < threads.length; i++) {
+        int loops = atLeast(100);
+        threads[i] =
+            new Thread("t" + i) {

Review comment:
       Better practice to pass a Runnable instead of overriding Thread.run directly.

##########
File path: lucene/core/src/test/org/apache/lucene/util/TestBytesRefHash.java
##########
@@ -267,6 +270,44 @@ public void testFind() throws Exception {
     }
   }
 
+  @Test
+  public void testConcurrentFind() throws Exception {
+    int num = atLeast(2);
+    for (int j = 0; j < num; j++) {
+      List<String> strings = new ArrayList<>();

Review comment:
       Can init to the proper size at construction time.

##########
File path: lucene/core/src/test/org/apache/lucene/util/TestBytesRefHash.java
##########
@@ -267,6 +270,44 @@ public void testFind() throws Exception {
     }
   }
 
+  @Test
+  public void testConcurrentFind() throws Exception {
+    int num = atLeast(2);
+    for (int j = 0; j < num; j++) {
+      List<String> strings = new ArrayList<>();
+      for (int i = 0; i < 797; i++) {
+        final String str = TestUtil.randomRealisticUnicodeString(random(), 1, 1000);
+        hash.add(new BytesRef(str));
+        assertTrue(strings.add(str));
+      }
+
+      AtomicInteger miss = new AtomicInteger();
+      Thread[] threads = new Thread[10];
+      for (int i = 0; i < threads.length; i++) {
+        int loops = atLeast(100);
+        threads[i] =
+            new Thread("t" + i) {
+              public void run() {

Review comment:
       I suspect you need a latch here or something so that the threads will all run concurrently, otherwise they are likely to start and run in series.




----------------------------------------------------------------
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.

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