From 65b2f7a5c6480f5c99c846d758239e2935ac1606 Mon Sep 17 00:00:00 2001
From: David <dmollitor@apache.org>
Date: Mon, 30 Sep 2024 23:48:59 -0400
Subject: [PATCH] AVRO-4069: Remove Reader String Cache from Generic Datum
 Reader

---
 .../avro/generic/GenericDatumReader.java      | 25 ++++---------
 .../avro/generic/TestGenericDatumReader.java  | 37 +------------------
 2 files changed, 8 insertions(+), 54 deletions(-)

diff --git a/lang/java/avro/src/main/java/org/apache/avro/generic/GenericDatumReader.java b/lang/java/avro/src/main/java/org/apache/avro/generic/GenericDatumReader.java
index 3c5d1316cb3..c1bc7494af3 100644
--- a/lang/java/avro/src/main/java/org/apache/avro/generic/GenericDatumReader.java
+++ b/lang/java/avro/src/main/java/org/apache/avro/generic/GenericDatumReader.java
@@ -452,7 +452,7 @@ protected Object newMap(Object old, int size) {
    * representation. By default, this calls {@link #readString(Object,Decoder)}.
    */
   protected Object readString(Object old, Schema expected, Decoder in) throws IOException {
-    Class stringClass = this.getReaderCache().getStringClass(expected);
+    Class stringClass = this.findStringClass(expected);
     if (stringClass == String.class) {
       return in.readString();
     }
@@ -490,12 +490,10 @@ protected Class findStringClass(Schema schema) {
     if (name == null)
       return CharSequence.class;
 
-    switch (GenericData.StringType.valueOf(name)) {
-    case String:
+    if (GenericData.StringType.String.name().equals(name)) {
       return String.class;
-    default:
-      return CharSequence.class;
     }
+    return CharSequence.class;
   }
 
   /**
@@ -529,14 +527,10 @@ public boolean equals(Object obj) {
 
   // VisibleForTesting
   static class ReaderCache {
-    private final Map<IdentitySchemaKey, Class> stringClassCache = new ConcurrentHashMap<>();
-
-    private final Map<Class, Function<String, Object>> stringCtorCache = new ConcurrentHashMap<>();
+    private final Map<Class, Function<String, Object>> stringCtorCache;
 
-    private final Function<Schema, Class> findStringClass;
-
-    public ReaderCache(Function<Schema, Class> findStringClass) {
-      this.findStringClass = findStringClass;
+    public ReaderCache() {
+      this.stringCtorCache = new ConcurrentHashMap<>();
     }
 
     public Object newInstanceFromString(Class c, String s) {
@@ -561,14 +555,9 @@ private Function<String, Object> buildFunction(Class c) {
         }
       };
     }
-
-    public Class getStringClass(final Schema s) {
-      final IdentitySchemaKey key = new IdentitySchemaKey(s);
-      return this.stringClassCache.computeIfAbsent(key, (IdentitySchemaKey k) -> this.findStringClass.apply(k.schema));
-    }
   }
 
-  private final ReaderCache readerCache = new ReaderCache(this::findStringClass);
+  private final ReaderCache readerCache = new ReaderCache();
 
   // VisibleForTesting
   ReaderCache getReaderCache() {
diff --git a/lang/java/avro/src/test/java/org/apache/avro/generic/TestGenericDatumReader.java b/lang/java/avro/src/test/java/org/apache/avro/generic/TestGenericDatumReader.java
index f74dab95b0f..b1ba2a45d50 100644
--- a/lang/java/avro/src/test/java/org/apache/avro/generic/TestGenericDatumReader.java
+++ b/lang/java/avro/src/test/java/org/apache/avro/generic/TestGenericDatumReader.java
@@ -23,8 +23,6 @@
 import java.util.Arrays;
 import java.util.List;
 import java.util.Random;
-import java.util.stream.Collectors;
-import java.util.stream.IntStream;
 
 import org.apache.avro.Schema;
 import org.junit.jupiter.api.Test;
@@ -33,27 +31,9 @@ public class TestGenericDatumReader {
 
   private static final Random r = new Random(System.currentTimeMillis());
 
-  @Test
-  void readerCache() {
-    final GenericDatumReader.ReaderCache cache = new GenericDatumReader.ReaderCache(this::findStringClass);
-    List<Thread> threads = IntStream.rangeClosed(1, 200).mapToObj((int index) -> {
-      final Schema schema = TestGenericDatumReader.this.build(index);
-      final WithSchema s = new WithSchema(schema, cache);
-      return (Runnable) () -> s.test();
-    }).map(Thread::new).collect(Collectors.toList());
-    threads.forEach(Thread::start);
-    threads.forEach((Thread t) -> {
-      try {
-        t.join();
-      } catch (InterruptedException e) {
-        throw new RuntimeException(e);
-      }
-    });
-  }
-
   @Test
   void newInstanceFromString() {
-    final GenericDatumReader.ReaderCache cache = new GenericDatumReader.ReaderCache(this::findStringClass);
+    final GenericDatumReader.ReaderCache cache = new GenericDatumReader.ReaderCache();
 
     Object object = cache.newInstanceFromString(StringBuilder.class, "Hello");
     assertEquals(StringBuilder.class, object.getClass());
@@ -62,21 +42,6 @@ void newInstanceFromString() {
 
   }
 
-  static class WithSchema {
-    private final Schema schema;
-
-    private final GenericDatumReader.ReaderCache cache;
-
-    public WithSchema(Schema schema, GenericDatumReader.ReaderCache cache) {
-      this.schema = schema;
-      this.cache = cache;
-    }
-
-    public void test() {
-      this.cache.getStringClass(schema);
-    }
-  }
-
   private List<Schema> list = new ArrayList<>();
 
   private Schema build(int index) {