From c6f754e5bcbcdc0943bb2f4477f2974d1ba42d57 Mon Sep 17 00:00:00 2001 From: Jason Koch Date: Fri, 9 Jan 2026 18:17:45 -0800 Subject: [PATCH 01/15] perf: cache AZ to Region lookup --- .../netflix/discovery/AbstractAzToRegionMapper.java | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/eureka-client/src/main/java/com/netflix/discovery/AbstractAzToRegionMapper.java b/eureka-client/src/main/java/com/netflix/discovery/AbstractAzToRegionMapper.java index 49c8e19e79..977ff23af6 100644 --- a/eureka-client/src/main/java/com/netflix/discovery/AbstractAzToRegionMapper.java +++ b/eureka-client/src/main/java/com/netflix/discovery/AbstractAzToRegionMapper.java @@ -40,6 +40,7 @@ public List get() { }); private final Map availabilityZoneVsRegion = new ConcurrentHashMap(); + private final Map parsedAzCache = new ConcurrentHashMap(); private String[] regionsToFetch; protected AbstractAzToRegionMapper(EurekaClientConfig clientConfig) { @@ -53,6 +54,7 @@ public synchronized void setRegionsToFetch(String[] regionsToFetch) { this.regionsToFetch = regionsToFetch; logger.info("Fetching availability zone to region mapping for regions {}", (Object) regionsToFetch); availabilityZoneVsRegion.clear(); + parsedAzCache.clear(); for (String remoteRegion : regionsToFetch) { Set availabilityZones = getZonesForARegion(remoteRegion); if (null == availabilityZones @@ -83,6 +85,7 @@ public synchronized void setRegionsToFetch(String[] regionsToFetch) { } else { logger.info("Regions to fetch is null. Erasing older mapping if any."); availabilityZoneVsRegion.clear(); + parsedAzCache.clear(); this.regionsToFetch = EMPTY_STR_ARRAY; } } @@ -96,9 +99,15 @@ public synchronized void setRegionsToFetch(String[] regionsToFetch) { @Override public String getRegionForAvailabilityZone(String availabilityZone) { - String region = availabilityZoneVsRegion.get(availabilityZone); + String region = parsedAzCache.get(availabilityZone); if (null == region) { - return parseAzToGetRegion(availabilityZone); + region = availabilityZoneVsRegion.get(availabilityZone); + if (null == region) { + region = parseAzToGetRegion(availabilityZone); + } + if (region != null) { + parsedAzCache.put(availabilityZone, region); + } } return region; } From 52dafa8d1b61164ca8bf1c437d316274e9875aa4 Mon Sep 17 00:00:00 2001 From: Jason Koch Date: Fri, 9 Jan 2026 09:22:06 -0800 Subject: [PATCH 02/15] perf: avoid Optional allocation in EurekaJacksonCodec hot loop --- .../com/netflix/discovery/converters/EurekaJacksonCodec.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/eureka-client/src/main/java/com/netflix/discovery/converters/EurekaJacksonCodec.java b/eureka-client/src/main/java/com/netflix/discovery/converters/EurekaJacksonCodec.java index c3c21039f2..8584e26eb5 100644 --- a/eureka-client/src/main/java/com/netflix/discovery/converters/EurekaJacksonCodec.java +++ b/eureka-client/src/main/java/com/netflix/discovery/converters/EurekaJacksonCodec.java @@ -638,7 +638,9 @@ public InstanceInfo deserialize(JsonParser jp, DeserializationContext context) t String key = intern.apply(jp, CacheScope.GLOBAL_SCOPE); jsonToken = jp.nextToken(); String value = intern.apply(jp, CacheScope.APPLICATION_SCOPE ); - metadataMap = Optional.ofNullable(metadataMap).orElseGet(METADATA_MAP_SUPPLIER); + if (metadataMap == null) { + metadataMap = METADATA_MAP_SUPPLIER.get(); + } metadataMap.put(key, value); } }; From 4c2dd1fdd22aed29019a5a3ebd547f0bcb6453d9 Mon Sep 17 00:00:00 2001 From: Jason Koch Date: Fri, 9 Jan 2026 10:01:44 -0800 Subject: [PATCH 03/15] perf: avoid Optional in Application getInstances --- .../main/java/com/netflix/discovery/shared/Application.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/eureka-client/src/main/java/com/netflix/discovery/shared/Application.java b/eureka-client/src/main/java/com/netflix/discovery/shared/Application.java index e069298ca5..3415380c0d 100644 --- a/eureka-client/src/main/java/com/netflix/discovery/shared/Application.java +++ b/eureka-client/src/main/java/com/netflix/discovery/shared/Application.java @@ -135,7 +135,11 @@ public void removeInstance(InstanceInfo i) { */ @JsonProperty("instance") public List getInstances() { - return Optional.ofNullable(shuffledInstances.get()).orElseGet(this::getInstancesAsIsFromEureka); + List instances = shuffledInstances.get(); + if (instances == null) { + instances = this.getInstancesAsIsFromEureka(); + } + return instances; } /** From 1a3d55c970d7deb9426c7da70c2d8fc6eeaf6692 Mon Sep 17 00:00:00 2001 From: Jason Koch Date: Fri, 9 Jan 2026 12:11:35 -0800 Subject: [PATCH 04/15] perf: avoid allocations in populateInstanceCountMap loop --- .../netflix/discovery/shared/Application.java | 13 +++++++++++++ .../discovery/shared/Applications.java | 19 ++++++++++++++----- 2 files changed, 27 insertions(+), 5 deletions(-) diff --git a/eureka-client/src/main/java/com/netflix/discovery/shared/Application.java b/eureka-client/src/main/java/com/netflix/discovery/shared/Application.java index 3415380c0d..020476ed09 100644 --- a/eureka-client/src/main/java/com/netflix/discovery/shared/Application.java +++ b/eureka-client/src/main/java/com/netflix/discovery/shared/Application.java @@ -28,6 +28,7 @@ import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.atomic.AtomicReference; +import java.util.function.Consumer; import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonIgnore; @@ -156,6 +157,18 @@ public List getInstancesAsIsFromEureka() { } } + /** + * Iterate over instances without creating a defensive copy. + * Package-private to avoid exposing unsynchronized iteration to external callers. + * Callers should be sure that this is a quick iteration. + */ + void forEachInstance(Consumer consumer) { + synchronized (instances) { + for (InstanceInfo info : instances) { + consumer.accept(info); + } + } + } /** * Get the instance info that matches the given id. diff --git a/eureka-client/src/main/java/com/netflix/discovery/shared/Applications.java b/eureka-client/src/main/java/com/netflix/discovery/shared/Applications.java index 80dbb88a1b..abe454f129 100644 --- a/eureka-client/src/main/java/com/netflix/discovery/shared/Applications.java +++ b/eureka-client/src/main/java/com/netflix/discovery/shared/Applications.java @@ -32,6 +32,7 @@ import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicLong; import java.util.concurrent.atomic.AtomicReference; +import java.util.function.Consumer; import java.util.stream.Collectors; import com.fasterxml.jackson.annotation.JsonCreator; @@ -244,11 +245,19 @@ public String getReconcileHashCode() { * the map to populate */ public void populateInstanceCountMap(Map instanceCountMap) { - for (Application app : this.getRegisteredApplications()) { - for (InstanceInfo info : app.getInstancesAsIsFromEureka()) { - AtomicInteger instanceCount = instanceCountMap.computeIfAbsent(info.getStatus().name(), - k -> new AtomicInteger(0)); - instanceCount.incrementAndGet(); + // accrue here as lightweight as possible + int[] statusCounts = new int[InstanceStatus.values().length]; + Consumer countByStatus = info -> statusCounts[info.getStatus().ordinal()]++; + for (Application app : this.applications) { + app.forEachInstance(countByStatus); + } + + // now convert it over to the API form in a single pass + for (InstanceStatus status : InstanceStatus.values()) { + int count = statusCounts[status.ordinal()]; + if (count > 0) { + instanceCountMap.computeIfAbsent(status.name(), k -> new AtomicInteger(0)) + .addAndGet(count); } } } From de8deaaafeaeae3aa6375fa5ed76d39578c717f5 Mon Sep 17 00:00:00 2001 From: Jason Koch Date: Fri, 9 Jan 2026 18:12:12 -0800 Subject: [PATCH 05/15] perf: avoid allocations when checking if Applications is empty --- .../java/com/netflix/discovery/DiscoveryClient.java | 4 ++-- .../com/netflix/discovery/shared/Applications.java | 11 +++++++++++ .../netflix/eureka/registry/RemoteRegionRegistry.java | 4 ++-- 3 files changed, 15 insertions(+), 4 deletions(-) diff --git a/eureka-client/src/main/java/com/netflix/discovery/DiscoveryClient.java b/eureka-client/src/main/java/com/netflix/discovery/DiscoveryClient.java index e04a778b1a..53b930e27e 100644 --- a/eureka-client/src/main/java/com/netflix/discovery/DiscoveryClient.java +++ b/eureka-client/src/main/java/com/netflix/discovery/DiscoveryClient.java @@ -1001,7 +1001,7 @@ private boolean fetchRegistry(boolean forceFullRegistryFetch) { || (!Strings.isNullOrEmpty(clientConfig.getRegistryRefreshSingleVipAddress())) || forceFullRegistryFetch || (applications == null) - || (applications.getRegisteredApplications().size() == 0) + || applications.isRegisteredApplicationsEmpty() || (applications.getVersion() == -1)) //Client application does not have latest library supporting delta { logger.info("Disable delta property : {}", clientConfig.shouldDisableDelta()); @@ -1009,7 +1009,7 @@ private boolean fetchRegistry(boolean forceFullRegistryFetch) { logger.info("Force full registry fetch : {}", forceFullRegistryFetch); logger.info("Application is null : {}", (applications == null)); logger.info("Registered Applications size is zero : {}", - (applications.getRegisteredApplications().size() == 0)); + applications.isRegisteredApplicationsEmpty()); logger.info("Application version is -1: {}", (applications.getVersion() == -1)); getAndStoreFullRegistry(); } else { diff --git a/eureka-client/src/main/java/com/netflix/discovery/shared/Applications.java b/eureka-client/src/main/java/com/netflix/discovery/shared/Applications.java index abe454f129..0e0a1876f0 100644 --- a/eureka-client/src/main/java/com/netflix/discovery/shared/Applications.java +++ b/eureka-client/src/main/java/com/netflix/discovery/shared/Applications.java @@ -138,6 +138,17 @@ public List getRegisteredApplications() { return new ArrayList(this.applications); } + /** + * Returns whether there are any registered applications. + * This is more efficient than {@code getRegisteredApplications().isEmpty()} + * as it avoids creating a defensive copy. + * + * @return true if there are no registered applications + */ + public boolean isRegisteredApplicationsEmpty() { + return this.applications.isEmpty(); + } + /** * Gets the registered application for the given * application name. diff --git a/eureka-core/src/main/java/com/netflix/eureka/registry/RemoteRegionRegistry.java b/eureka-core/src/main/java/com/netflix/eureka/registry/RemoteRegionRegistry.java index 130e0e97ad..20753fad1b 100644 --- a/eureka-core/src/main/java/com/netflix/eureka/registry/RemoteRegionRegistry.java +++ b/eureka-core/src/main/java/com/netflix/eureka/registry/RemoteRegionRegistry.java @@ -235,10 +235,10 @@ private boolean fetchRegistry() { // If the delta is disabled or if it is the first time, get all applications if (serverConfig.shouldDisableDeltaForRemoteRegions() || (getApplications() == null) - || (getApplications().getRegisteredApplications().size() == 0)) { + || getApplications().isRegisteredApplicationsEmpty()) { logger.info("Disable delta property : {}", serverConfig.shouldDisableDeltaForRemoteRegions()); logger.info("Application is null : {}", getApplications() == null); - logger.info("Registered Applications size is zero : {}", getApplications().getRegisteredApplications().isEmpty()); + logger.info("Registered Applications size is zero : {}", getApplications().isRegisteredApplicationsEmpty()); success = storeFullRegistry(); } else { success = fetchAndStoreDelta(); From 887e7d96f6b4262bca0ed6a21ca7944d31222706 Mon Sep 17 00:00:00 2001 From: Jason Koch Date: Wed, 7 Jan 2026 14:26:43 -0800 Subject: [PATCH 06/15] perf: Reduce allocations in DeserializerStringCache by reusing parser buffer --- .../util/DeserializerStringCache.java | 344 ++++++++++-------- .../util/DeserializerStringCacheTest.java | 249 +++++++++++-- 2 files changed, 408 insertions(+), 185 deletions(-) diff --git a/eureka-client/src/main/java/com/netflix/discovery/util/DeserializerStringCache.java b/eureka-client/src/main/java/com/netflix/discovery/util/DeserializerStringCache.java index 013e8ddbbf..e9375737f4 100644 --- a/eureka-client/src/main/java/com/netflix/discovery/util/DeserializerStringCache.java +++ b/eureka-client/src/main/java/com/netflix/discovery/util/DeserializerStringCache.java @@ -45,6 +45,10 @@ public enum CacheScope { private final Map applicationCache; private final int lengthLimit = LENGTH_LIMIT; + // Reusable lookup buffers to avoid allocation on cache hits (single-threaded usage) + private final MutableArrayCharBuffer parserLookupBuffer = new MutableArrayCharBuffer(); + private final MutableStringCharBuffer stringLookupBuffer = new MutableStringCharBuffer(); + /** * adds a new DeserializerStringCache to the passed-in ObjectReader * @@ -207,44 +211,17 @@ public String apply(final JsonParser jp, CacheScope cacheScope) throws IOExcepti * @throws IOException */ public String apply(final JsonParser jp, CacheScope cacheScope, Supplier source) throws IOException { - return apply(CharBuffer.wrap(jp, source), cacheScope); - } - - /** - * returns a String that may be interned at app-scope to reduce heap - * consumption - * - * @param charValue - * @return a possibly interned String - */ - public String apply(final CharBuffer charValue) { - return apply(charValue, CacheScope.APPLICATION_SCOPE); - } - - /** - * returns a object of type T that may be interned at the specified scope to - * reduce heap consumption - * - * @param charValue - * @param cacheScope - * @param trabsform - * @return a possibly interned instance of T - */ - public String apply(CharBuffer charValue, CacheScope cacheScope) { - int keyLength = charValue.length(); - if ((lengthLimit < 0 || keyLength <= lengthLimit)) { + parserLookupBuffer.reset(jp, source); + int keyLength = parserLookupBuffer.length(); + if (lengthLimit < 0 || keyLength <= lengthLimit) { Map cache = (cacheScope == CacheScope.GLOBAL_SCOPE) ? globalCache : applicationCache; - String value = cache.get(charValue); + String value = cache.get(parserLookupBuffer); if (value == null) { - value = charValue.consume((k, v) -> { - cache.put(k, v); - }); - } else { - // System.out.println("cache hit"); + value = parserLookupBuffer.consume(cache::put); } return value; } - return charValue.toString(); + return parserLookupBuffer.toString(); } /** @@ -269,11 +246,15 @@ public String apply(final String stringValue) { */ public String apply(final String stringValue, CacheScope cacheScope) { if (stringValue != null && (lengthLimit < 0 || stringValue.length() <= lengthLimit)) { - return (String) (cacheScope == CacheScope.GLOBAL_SCOPE ? globalCache : applicationCache) - .computeIfAbsent(CharBuffer.wrap(stringValue), s -> { - logger.trace(" (string) writing new interned value {} into {} cache scope", stringValue, cacheScope); - return stringValue; - }); + stringLookupBuffer.reset(stringValue); + Map cache = (cacheScope == CacheScope.GLOBAL_SCOPE) ? globalCache : applicationCache; + String value = cache.get(stringLookupBuffer); + if (value == null) { + logger.trace(" (string) writing new interned value {} into {} cache scope", stringValue, cacheScope); + cache.put(new CharBuffer.StringCharBuffer(stringValue), stringValue); + value = stringValue; + } + return value; } return stringValue; } @@ -285,18 +266,6 @@ public int size() { private interface CharBuffer { static final int DEFAULT_VARIANT = -1; - public static CharBuffer wrap(JsonParser source, Supplier stringSource) throws IOException { - return new ArrayCharBuffer(source, stringSource); - } - - public static CharBuffer wrap(JsonParser source) throws IOException { - return new ArrayCharBuffer(source); - } - - public static CharBuffer wrap(String source) { - return new StringCharBuffer(source); - } - String consume(BiConsumer valueConsumer); int length(); @@ -305,118 +274,20 @@ public static CharBuffer wrap(String source) { OfInt chars(); - static class ArrayCharBuffer implements CharBuffer { - private final char[] source; - private final int offset; - private final int length; - private final Supplier valueTransform; - private final int variant; - private final int hash; - - ArrayCharBuffer(JsonParser source) throws IOException { - this(source, null); - } - - ArrayCharBuffer(JsonParser source, Supplier valueTransform) throws IOException { - this.source = source.getTextCharacters(); - this.offset = source.getTextOffset(); - this.length = source.getTextLength(); - this.valueTransform = valueTransform; - this.variant = valueTransform == null ? DEFAULT_VARIANT : System.identityHashCode(valueTransform.getClass()); - this.hash = 31 * arrayHash(this.source, offset, length) + variant; - } - - @Override - public int length() { - return length; - } - - @Override - public int variant() { - return variant; - } - - @Override - public int hashCode() { - return hash; - } - - @Override - public boolean equals(Object other) { - if (other instanceof CharBuffer) { - CharBuffer otherBuffer = (CharBuffer) other; - if (otherBuffer.length() == length) { - if (otherBuffer.variant() == variant) { - OfInt otherText = otherBuffer.chars(); - for (int i = offset; i < length; i++) { - if (source[i] != otherText.nextInt()) { - return false; - } - } - return true; - } - } - } - return false; - } - - @Override - public OfInt chars() { - return new OfInt() { - int index = offset; - int limit = index + length; - - @Override - public boolean hasNext() { - return index < limit; - } - - @Override - public int nextInt() { - return source[index++]; - } - }; - } - - @Override - public String toString() { - return valueTransform == null ? new String(this.source, offset, length) : valueTransform.get(); - } - - @Override - public String consume(BiConsumer valueConsumer) { - String key = new String(this.source, offset, length); - String value = valueTransform == null ? key : valueTransform.get(); - valueConsumer.accept(new StringCharBuffer(key, variant), value); - return value; - } - - private static int arrayHash(char[] a, int offset, int length) { - if (a == null) - return 0; - int result = 0; - int limit = offset + length; - for (int i = offset; i < limit; i++) { - result = 31 * result + a[i]; - } - return result; - } - } - static class StringCharBuffer implements CharBuffer { private final String source; private final int variant; private final int hashCode; StringCharBuffer(String source) { - this(source, -1); + this(source, DEFAULT_VARIANT); } StringCharBuffer(String source, int variant) { this.source = source; this.variant = variant; this.hashCode = 31 * source.hashCode() + variant; - } + } @Override public int hashCode() { @@ -483,4 +354,177 @@ public String consume(BiConsumer valueConsumer) { } } + + /** + * Mutable version of ArrayCharBuffer for reusable lookups. + * Reset before each use to avoid allocations on cache hits. + */ + private static class MutableArrayCharBuffer implements CharBuffer { + private char[] source; + private int offset; + private int length; + private Supplier valueTransform; + private int variant; + private int hash; + + void reset(JsonParser jp, Supplier valueTransform) throws IOException { + this.source = jp.getTextCharacters(); + this.offset = jp.getTextOffset(); + this.length = jp.getTextLength(); + this.valueTransform = valueTransform; + this.variant = valueTransform == null ? DEFAULT_VARIANT : System.identityHashCode(valueTransform.getClass()); + this.hash = 31 * arrayHash(this.source, offset, length) + variant; + } + + @Override + public int length() { + return length; + } + + @Override + public int variant() { + return variant; + } + + @Override + public int hashCode() { + return hash; + } + + @Override + public boolean equals(Object other) { + if (other instanceof CharBuffer) { + CharBuffer otherBuffer = (CharBuffer) other; + if (otherBuffer.length() == length && otherBuffer.variant() == variant) { + OfInt otherText = otherBuffer.chars(); + for (int i = offset; i < offset + length; i++) { + if (source[i] != otherText.nextInt()) { + return false; + } + } + return true; + } + } + return false; + } + + @Override + public OfInt chars() { + return new OfInt() { + int index = offset; + int limit = index + length; + + @Override + public boolean hasNext() { + return index < limit; + } + + @Override + public int nextInt() { + return source[index++]; + } + }; + } + + @Override + public String toString() { + return valueTransform == null ? new String(this.source, offset, length) : valueTransform.get(); + } + + @Override + public String consume(BiConsumer valueConsumer) { + String key = new String(this.source, offset, length); + String value = valueTransform == null ? key : valueTransform.get(); + valueConsumer.accept(new CharBuffer.StringCharBuffer(key, variant), value); + return value; + } + + private static int arrayHash(char[] a, int offset, int length) { + if (a == null) + return 0; + int result = 0; + int limit = offset + length; + for (int i = offset; i < limit; i++) { + result = 31 * result + a[i]; + } + return result; + } + } + + /** + * Mutable version of StringCharBuffer for reusable lookups. + * Reset before each use to avoid allocations on cache hits. + */ + private static class MutableStringCharBuffer implements CharBuffer { + private String source; + private int hashCode; + + void reset(String source) { + this.source = source; + this.hashCode = 31 * source.hashCode() + DEFAULT_VARIANT; + } + + @Override + public int hashCode() { + return hashCode; + } + + @Override + public int variant() { + return DEFAULT_VARIANT; + } + + @Override + public boolean equals(Object other) { + if (other instanceof CharBuffer) { + CharBuffer otherBuffer = (CharBuffer) other; + if (otherBuffer.variant() == DEFAULT_VARIANT) { + int length = source.length(); + if (otherBuffer.length() == length) { + OfInt otherText = otherBuffer.chars(); + for (int i = 0; i < length; i++) { + if (source.charAt(i) != otherText.nextInt()) { + return false; + } + } + return true; + } + } + } + return false; + } + + @Override + public int length() { + return source.length(); + } + + @Override + public String toString() { + return source; + } + + @Override + public OfInt chars() { + return new OfInt() { + int index; + + @Override + public boolean hasNext() { + return index < source.length(); + } + + @Override + public int nextInt() { + return source.charAt(index++); + } + }; + } + + @Override + public String consume(BiConsumer valueConsumer) { + valueConsumer.accept(new CharBuffer.StringCharBuffer(source), source); + return source; + } + } } diff --git a/eureka-client/src/test/java/com/netflix/discovery/util/DeserializerStringCacheTest.java b/eureka-client/src/test/java/com/netflix/discovery/util/DeserializerStringCacheTest.java index 6cf9b18d7f..fe52b220a5 100644 --- a/eureka-client/src/test/java/com/netflix/discovery/util/DeserializerStringCacheTest.java +++ b/eureka-client/src/test/java/com/netflix/discovery/util/DeserializerStringCacheTest.java @@ -2,60 +2,239 @@ import java.io.IOException; import java.util.Arrays; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.function.Supplier; +import com.fasterxml.jackson.core.JsonFactory; import com.fasterxml.jackson.core.JsonParser; -import com.fasterxml.jackson.databind.DeserializationContext; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.ObjectReader; import com.netflix.discovery.util.DeserializerStringCache.CacheScope; import org.junit.Test; import static org.hamcrest.CoreMatchers.is; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotSame; +import static org.junit.Assert.assertSame; import static org.junit.Assert.assertThat; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; public class DeserializerStringCacheTest { + private static final JsonFactory JSON_FACTORY = new JsonFactory(); + + private static JsonParser createParser(String jsonValue) throws IOException { + // Create a real parser positioned at a string value + JsonParser parser = JSON_FACTORY.createParser("\"" + jsonValue + "\""); + parser.nextToken(); // Move to VALUE_STRING + return parser; + } + + private DeserializerStringCache createCache() { + ObjectReader reader = DeserializerStringCache.init(new ObjectMapper().reader()); + return (DeserializerStringCache) reader.getAttributes().getAttribute("deserInternCache"); + } + @Test public void testUppercaseConversionWithLowercasePreset() throws IOException { - DeserializationContext deserializationContext = mock(DeserializationContext.class); - DeserializerStringCache deserializerStringCache = DeserializerStringCache.from(deserializationContext); + DeserializerStringCache cache = createCache(); - String lowerCaseValue = deserializerStringCache.apply("value", CacheScope.APPLICATION_SCOPE); + String lowerCaseValue = cache.apply("value", CacheScope.APPLICATION_SCOPE); assertThat(lowerCaseValue, is("value")); - JsonParser jsonParser = mock(JsonParser.class); - when(jsonParser.getTextCharacters()).thenReturn(new char[] {'v', 'a', 'l', 'u', 'e'}); - when(jsonParser.getTextLength()).thenReturn(5); - - String upperCaseValue = deserializerStringCache.apply(jsonParser, CacheScope.APPLICATION_SCOPE, () -> "VALUE"); - assertThat(upperCaseValue, is("VALUE")); + try (JsonParser jsonParser = createParser("value")) { + String upperCaseValue = cache.apply(jsonParser, CacheScope.APPLICATION_SCOPE, () -> "VALUE"); + assertThat(upperCaseValue, is("VALUE")); + } } @Test public void testUppercaseConversionWithLongString() throws IOException { - DeserializationContext deserializationContext = mock(DeserializationContext.class); - DeserializerStringCache deserializerStringCache = DeserializerStringCache.from(deserializationContext); + DeserializerStringCache cache = createCache(); char[] lowercaseValue = new char[1024]; Arrays.fill(lowercaseValue, 'a'); + String longString = new String(lowercaseValue); + + try (JsonParser jsonParser = createParser(longString)) { + char[] expectedValueChars = new char[1024]; + Arrays.fill(expectedValueChars, 'A'); + String expectedValue = new String(expectedValueChars); + + String upperCaseValue = cache.apply(jsonParser, CacheScope.APPLICATION_SCOPE, + () -> longString.toUpperCase()); + assertThat(upperCaseValue, is(expectedValue)); + } + } + + @Test + public void testCacheHitReturnsIdenticalInstance() throws IOException { + DeserializerStringCache cache = createCache(); + + String first; + try (JsonParser p1 = createParser("testValue")) { + first = cache.apply(p1, CacheScope.APPLICATION_SCOPE); + } + + String second; + try (JsonParser p2 = createParser("testValue")) { + second = cache.apply(p2, CacheScope.APPLICATION_SCOPE); + } + + assertSame("Cache hit should return identical instance", first, second); + } - JsonParser jsonParser = mock(JsonParser.class); - when(jsonParser.getText()).thenReturn(new String(lowercaseValue)); - when(jsonParser.getTextCharacters()).thenReturn(lowercaseValue); - when(jsonParser.getTextOffset()).thenReturn(0); - when(jsonParser.getTextLength()).thenReturn(lowercaseValue.length); - - String upperCaseValue = deserializerStringCache.apply(jsonParser, CacheScope.APPLICATION_SCOPE, () -> { - try { - return jsonParser.getText().toUpperCase(); - } - catch(IOException ioe) { - // not likely from mock above - throw new IllegalStateException("mock threw unexpected exception", ioe); - } - }); - char[] expectedValueChars = new char[1024]; - Arrays.fill(expectedValueChars, 'A'); - String expectedValue = new String(expectedValueChars); - assertThat(upperCaseValue, is(expectedValue)); - } -} \ No newline at end of file + @Test + public void testCacheHitWithStringReturnsIdenticalInstance() throws IOException { + DeserializerStringCache cache = createCache(); + + String first = cache.apply(new String("testValue"), CacheScope.APPLICATION_SCOPE); + String second = cache.apply(new String("testValue"), CacheScope.APPLICATION_SCOPE); + + assertSame("Cache hit should return identical instance", first, second); + } + + @Test + public void testCacheHitAcrossParserAndString() throws IOException { + DeserializerStringCache cache = createCache(); + + String fromParser; + try (JsonParser parser = createParser("testValue")) { + fromParser = cache.apply(parser, CacheScope.APPLICATION_SCOPE); + } + String fromString = cache.apply(new String("testValue"), CacheScope.APPLICATION_SCOPE); + + assertSame("Cache should work across parser and string lookups", fromParser, fromString); + } + + @Test + public void testSupplierOnlyCalledOnCacheMiss() throws IOException { + DeserializerStringCache cache = createCache(); + AtomicInteger callCount = new AtomicInteger(0); + + Supplier countingSupplier = () -> { + callCount.incrementAndGet(); + return "TRANSFORMED"; + }; + + try (JsonParser p1 = createParser("value")) { + cache.apply(p1, CacheScope.APPLICATION_SCOPE, countingSupplier); + } + try (JsonParser p2 = createParser("value")) { + cache.apply(p2, CacheScope.APPLICATION_SCOPE, countingSupplier); + } + + assertEquals("Supplier should only be called once (on cache miss)", 1, callCount.get()); + } + + @Test + public void testGlobalScopeSurvivesApplicationScopeClear() throws IOException { + ObjectReader reader = DeserializerStringCache.init(new ObjectMapper().reader()); + DeserializerStringCache cache = (DeserializerStringCache) reader.getAttributes() + .getAttribute("deserInternCache"); + + String globalValue; + try (JsonParser p1 = createParser("globalKey")) { + globalValue = cache.apply(p1, CacheScope.GLOBAL_SCOPE); + } + + String appValue; + try (JsonParser p2 = createParser("appKey")) { + appValue = cache.apply(p2, CacheScope.APPLICATION_SCOPE); + } + + // Clear only application scope + DeserializerStringCache.clear(reader, CacheScope.APPLICATION_SCOPE); + + // Global should still return same instance + String globalAgain; + try (JsonParser p3 = createParser("globalKey")) { + globalAgain = cache.apply(p3, CacheScope.GLOBAL_SCOPE); + } + assertSame("Global value should survive application scope clear", globalValue, globalAgain); + + // Application scope was cleared, so this should be a new instance + String appAgain; + try (JsonParser p4 = createParser("appKey")) { + appAgain = cache.apply(p4, CacheScope.APPLICATION_SCOPE); + } + assertNotSame("Application value should be new after clear", appValue, appAgain); + assertEquals("Application value should have same content", appValue, appAgain); + } + + @Test + public void testGlobalScopeClearClearsBothScopes() throws IOException { + ObjectReader reader = DeserializerStringCache.init(new ObjectMapper().reader()); + DeserializerStringCache cache = (DeserializerStringCache) reader.getAttributes() + .getAttribute("deserInternCache"); + + String globalValue; + try (JsonParser p1 = createParser("globalKey")) { + globalValue = cache.apply(p1, CacheScope.GLOBAL_SCOPE); + } + + String appValue; + try (JsonParser p2 = createParser("appKey")) { + appValue = cache.apply(p2, CacheScope.APPLICATION_SCOPE); + } + + // Clear global scope (should clear both) + DeserializerStringCache.clear(reader, CacheScope.GLOBAL_SCOPE); + + String globalAgain; + try (JsonParser p3 = createParser("globalKey")) { + globalAgain = cache.apply(p3, CacheScope.GLOBAL_SCOPE); + } + + String appAgain; + try (JsonParser p4 = createParser("appKey")) { + appAgain = cache.apply(p4, CacheScope.APPLICATION_SCOPE); + } + + assertNotSame("Global value should be new after global clear", globalValue, globalAgain); + assertNotSame("Application value should be new after global clear", appValue, appAgain); + } + + @Test + public void testParserWithNonZeroOffset() throws IOException { + DeserializerStringCache cache = createCache(); + + // First cache "value" from a normal parse + String cached; + try (JsonParser p1 = createParser("value")) { + cached = cache.apply(p1, CacheScope.APPLICATION_SCOPE); + } + assertEquals("Should extract correct value", "value", cached); + + // Verify same value from different parse returns cached instance + String cachedAgain; + try (JsonParser p2 = createParser("value")) { + cachedAgain = cache.apply(p2, CacheScope.APPLICATION_SCOPE); + } + assertSame("Should match cache entry", cached, cachedAgain); + } + + @Test + public void testDifferentTransformsForSameKeyAreCachedSeparately() throws IOException { + DeserializerStringCache cache = createCache(); + + // Same raw key "app" but different transforms (identity vs toUpperCase) + String lowercase; + try (JsonParser p1 = createParser("app")) { + lowercase = cache.apply(p1, CacheScope.APPLICATION_SCOPE); + } + + // Use a different supplier class - this should create a different cache entry + // because the variant is based on the supplier class identity + class UpperCaseSupplier implements Supplier { + public String get() { return "APP"; } + } + + String uppercase; + try (JsonParser p2 = createParser("app")) { + uppercase = cache.apply(p2, CacheScope.APPLICATION_SCOPE, new UpperCaseSupplier()); + } + + assertEquals("Lowercase should be 'app'", "app", lowercase); + assertEquals("Uppercase should be 'APP'", "APP", uppercase); + assertNotSame("Different transforms should cache separately", lowercase, uppercase); + } +} From 47719b878965a3284ac7c5fb1a67f31c8060168f Mon Sep 17 00:00:00 2001 From: Jason Koch Date: Wed, 7 Jan 2026 16:45:12 -0800 Subject: [PATCH 07/15] perf: Avoid lambda capture in InstanceInfoDeserializer toUpperCase extract --- .../converters/EurekaJacksonCodec.java | 28 ++++++++----------- .../util/DeserializerStringCache.java | 15 ++++++---- .../util/DeserializerStringCacheTest.java | 26 ++++++++--------- 3 files changed, 34 insertions(+), 35 deletions(-) diff --git a/eureka-client/src/main/java/com/netflix/discovery/converters/EurekaJacksonCodec.java b/eureka-client/src/main/java/com/netflix/discovery/converters/EurekaJacksonCodec.java index 8584e26eb5..39bec5904d 100644 --- a/eureka-client/src/main/java/com/netflix/discovery/converters/EurekaJacksonCodec.java +++ b/eureka-client/src/main/java/com/netflix/discovery/converters/EurekaJacksonCodec.java @@ -424,6 +424,15 @@ protected void autoMarshalEligible(Object o, JsonGenerator jgen) { public static class InstanceInfoDeserializer extends JsonDeserializer { private static char[] BUF_AT_CLASS = "@class".toCharArray(); + /** Extract uppercase from current JsonParser cursor. Avoids lambda capture. */ + private static String toUpperCase(JsonParser jp) { + try { + return jp.getText().toUpperCase(); + } catch (IOException e) { + throw new RuntimeJsonMappingException(e.getMessage()); + } + } + enum InstanceInfoField { HOSTNAME(ELEM_HOST), INSTANCE_ID(ELEM_INSTANCE_ID), @@ -515,14 +524,7 @@ public InstanceInfo deserialize(JsonParser jp, DeserializationContext context) t break; case APP: builder.setAppNameForDeser( - intern.apply(jp, CacheScope.APPLICATION_SCOPE, - ()->{ - try { - return jp.getText().toUpperCase(); - } catch (IOException e) { - throw new RuntimeJsonMappingException(e.getMessage()); - } - })); + intern.apply(jp, CacheScope.APPLICATION_SCOPE, InstanceInfoDeserializer::toUpperCase)); break; case IP: builder.setIPAddr(intern.apply(jp)); @@ -590,14 +592,8 @@ public InstanceInfo deserialize(JsonParser jp, DeserializationContext context) t builder.setHealthCheckUrlsForDeser(null, intern.apply(jp.getText())); break; case APPGROUPNAME: - builder.setAppGroupNameForDeser(intern.apply(jp, CacheScope.GLOBAL_SCOPE, - ()->{ - try { - return jp.getText().toUpperCase(); - } catch (IOException e) { - throw new RuntimeJsonMappingException(e.getMessage()); - } - })); + builder.setAppGroupNameForDeser( + intern.apply(jp, CacheScope.GLOBAL_SCOPE, InstanceInfoDeserializer::toUpperCase)); break; case HOMEPAGEURL: builder.setHomePageUrlForDeser(intern.apply(jp.getText())); diff --git a/eureka-client/src/main/java/com/netflix/discovery/util/DeserializerStringCache.java b/eureka-client/src/main/java/com/netflix/discovery/util/DeserializerStringCache.java index e9375737f4..3b65648f07 100644 --- a/eureka-client/src/main/java/com/netflix/discovery/util/DeserializerStringCache.java +++ b/eureka-client/src/main/java/com/netflix/discovery/util/DeserializerStringCache.java @@ -207,11 +207,12 @@ public String apply(final JsonParser jp, CacheScope cacheScope) throws IOExcepti * * @param jp * @param cacheScope + * @param transform optional transform function applied to the parser text on cache miss * @return a possibly interned String * @throws IOException */ - public String apply(final JsonParser jp, CacheScope cacheScope, Supplier source) throws IOException { - parserLookupBuffer.reset(jp, source); + public String apply(final JsonParser jp, CacheScope cacheScope, Function transform) throws IOException { + parserLookupBuffer.reset(jp, transform); int keyLength = parserLookupBuffer.length(); if (lengthLimit < 0 || keyLength <= lengthLimit) { Map cache = (cacheScope == CacheScope.GLOBAL_SCOPE) ? globalCache : applicationCache; @@ -360,14 +361,16 @@ public String consume(BiConsumer valueConsumer) { * Reset before each use to avoid allocations on cache hits. */ private static class MutableArrayCharBuffer implements CharBuffer { + private JsonParser jp; private char[] source; private int offset; private int length; - private Supplier valueTransform; + private Function valueTransform; private int variant; private int hash; - void reset(JsonParser jp, Supplier valueTransform) throws IOException { + void reset(JsonParser jp, Function valueTransform) throws IOException { + this.jp = jp; this.source = jp.getTextCharacters(); this.offset = jp.getTextOffset(); this.length = jp.getTextLength(); @@ -428,13 +431,13 @@ public int nextInt() { @Override public String toString() { - return valueTransform == null ? new String(this.source, offset, length) : valueTransform.get(); + return valueTransform == null ? new String(this.source, offset, length) : valueTransform.apply(jp); } @Override public String consume(BiConsumer valueConsumer) { String key = new String(this.source, offset, length); - String value = valueTransform == null ? key : valueTransform.get(); + String value = valueTransform == null ? key : valueTransform.apply(jp); valueConsumer.accept(new CharBuffer.StringCharBuffer(key, variant), value); return value; } diff --git a/eureka-client/src/test/java/com/netflix/discovery/util/DeserializerStringCacheTest.java b/eureka-client/src/test/java/com/netflix/discovery/util/DeserializerStringCacheTest.java index fe52b220a5..cfd1bac254 100644 --- a/eureka-client/src/test/java/com/netflix/discovery/util/DeserializerStringCacheTest.java +++ b/eureka-client/src/test/java/com/netflix/discovery/util/DeserializerStringCacheTest.java @@ -3,7 +3,7 @@ import java.io.IOException; import java.util.Arrays; import java.util.concurrent.atomic.AtomicInteger; -import java.util.function.Supplier; +import java.util.function.Function; import com.fasterxml.jackson.core.JsonFactory; import com.fasterxml.jackson.core.JsonParser; @@ -42,7 +42,7 @@ public void testUppercaseConversionWithLowercasePreset() throws IOException { assertThat(lowerCaseValue, is("value")); try (JsonParser jsonParser = createParser("value")) { - String upperCaseValue = cache.apply(jsonParser, CacheScope.APPLICATION_SCOPE, () -> "VALUE"); + String upperCaseValue = cache.apply(jsonParser, CacheScope.APPLICATION_SCOPE, jp -> "VALUE"); assertThat(upperCaseValue, is("VALUE")); } } @@ -60,7 +60,7 @@ public void testUppercaseConversionWithLongString() throws IOException { String expectedValue = new String(expectedValueChars); String upperCaseValue = cache.apply(jsonParser, CacheScope.APPLICATION_SCOPE, - () -> longString.toUpperCase()); + jp -> longString.toUpperCase()); assertThat(upperCaseValue, is(expectedValue)); } } @@ -106,23 +106,23 @@ public void testCacheHitAcrossParserAndString() throws IOException { } @Test - public void testSupplierOnlyCalledOnCacheMiss() throws IOException { + public void testTransformOnlyCalledOnCacheMiss() throws IOException { DeserializerStringCache cache = createCache(); AtomicInteger callCount = new AtomicInteger(0); - Supplier countingSupplier = () -> { + Function countingTransform = jp -> { callCount.incrementAndGet(); return "TRANSFORMED"; }; try (JsonParser p1 = createParser("value")) { - cache.apply(p1, CacheScope.APPLICATION_SCOPE, countingSupplier); + cache.apply(p1, CacheScope.APPLICATION_SCOPE, countingTransform); } try (JsonParser p2 = createParser("value")) { - cache.apply(p2, CacheScope.APPLICATION_SCOPE, countingSupplier); + cache.apply(p2, CacheScope.APPLICATION_SCOPE, countingTransform); } - assertEquals("Supplier should only be called once (on cache miss)", 1, callCount.get()); + assertEquals("Transform should only be called once (on cache miss)", 1, callCount.get()); } @Test @@ -222,15 +222,15 @@ public void testDifferentTransformsForSameKeyAreCachedSeparately() throws IOExce lowercase = cache.apply(p1, CacheScope.APPLICATION_SCOPE); } - // Use a different supplier class - this should create a different cache entry - // because the variant is based on the supplier class identity - class UpperCaseSupplier implements Supplier { - public String get() { return "APP"; } + // Use a different function class - this should create a different cache entry + // because the variant is based on the function class identity + class UpperCaseTransform implements Function { + public String apply(JsonParser jp) { return "APP"; } } String uppercase; try (JsonParser p2 = createParser("app")) { - uppercase = cache.apply(p2, CacheScope.APPLICATION_SCOPE, new UpperCaseSupplier()); + uppercase = cache.apply(p2, CacheScope.APPLICATION_SCOPE, new UpperCaseTransform()); } assertEquals("Lowercase should be 'app'", "app", lowercase); From a5b15d028b0a74125909c95a868711e99b000725 Mon Sep 17 00:00:00 2001 From: Jason Koch Date: Thu, 8 Jan 2026 15:02:02 -0800 Subject: [PATCH 08/15] perf: Reduce String and byte[] allocations in Applications addInstanceToMap We can special-case the common situation to avoid a lot of the allocations. --- .../discovery/shared/Applications.java | 75 ++++++++++++++++--- 1 file changed, 66 insertions(+), 9 deletions(-) diff --git a/eureka-client/src/main/java/com/netflix/discovery/shared/Applications.java b/eureka-client/src/main/java/com/netflix/discovery/shared/Applications.java index 0e0a1876f0..bdc8031dcf 100644 --- a/eureka-client/src/main/java/com/netflix/discovery/shared/Applications.java +++ b/eureka-client/src/main/java/com/netflix/discovery/shared/Applications.java @@ -66,10 +66,38 @@ @JsonRootName("applications") public class Applications { private static class VipIndexSupport { - final AbstractQueue instances = new ConcurrentLinkedQueue<>(); + // Progressive list: emptyList (0) -> singletonList (1) -> ArrayList (2+) + // This avoids CLQ and Node allocations. 56% of VIPs have exactly 1 instance. + private List instances = Collections.emptyList(); final AtomicLong roundRobinIndex = new AtomicLong(0); final AtomicReference> vipList = new AtomicReference<>(Collections.emptyList()); + void addInstance(InstanceInfo info) { + int size = instances.size(); + if (size == 0) { + // 0 -> 1: use singletonList (56% of VIPs stop here) + instances = Collections.singletonList(info); + } else if (size == 1) { + // 1 -> 2: transition singletonList to ArrayList + InstanceInfo first = instances.get(0); + ArrayList list = new ArrayList<>(12); + list.add(first); + list.add(info); + instances = list; + } else { + // 2+ -> n: append to ArrayList + ((ArrayList) instances).add(info); + } + } + + int instanceCount() { + return instances.size(); + } + + List getInstances() { + return instances; + } + public AtomicLong getRoundRobinIndex() { return roundRobinIndex; } @@ -372,7 +400,7 @@ private void shuffleAndFilterInstances(Map srcMap, bool Random shuffleRandom = new Random(); for (Map.Entry entries : srcMap.entrySet()) { VipIndexSupport vipIndexSupport = entries.getValue(); - AbstractQueue vipInstances = vipIndexSupport.instances; + List vipInstances = vipIndexSupport.getInstances(); final List filteredInstances; if (filterUpInstances) { filteredInstances = vipInstances.stream().filter(ii -> ii.getStatus() == InstanceStatus.UP) @@ -390,16 +418,45 @@ private void shuffleAndFilterInstances(Map srcMap, bool * Add the instance to the given map based if the vip address matches with * that of the instance. Note that an instance can be mapped to multiple vip * addresses. - * */ private void addInstanceToMap(InstanceInfo info, String vipAddresses, Map vipMap) { - if (vipAddresses != null) { - String[] vipAddressArray = vipAddresses.toUpperCase(Locale.ROOT).split(","); - for (String vipAddress : vipAddressArray) { - VipIndexSupport vis = vipMap.computeIfAbsent(vipAddress, k -> new VipIndexSupport()); - vis.instances.add(info); - } + // This code path is quite hot on allocations. We apply common-case optimizations to minimize allocations. + // Gathered statistics from a real cluster: + // | Metric | Test | Prod | + // |-----------------------|--------|---------| + // | Total entries | N | 2x N | + // | Single VIP (no comma) | 91.1% | 91.7% | + // | 2 VIPs | 6.9% | 5.9% | + // | 3+ VIPs | 0.4% | 0.7% | + // | Empty | 1.6% | 1.7% | + // | Max VIPs per entry | 7 | 13 | + // | Avg string length | 29.5 | 25.8 | + // | Max string length | 204 | 468 | + + if (vipAddresses == null || vipAddresses.isEmpty()) { + return; } + + String upper = vipAddresses.toUpperCase(Locale.ROOT); + + // Fast path (91.7% of cases) single VIP: no split() -> byte[], no substring() -> String + int commaIndex = upper.indexOf(','); + if (commaIndex == -1) { + vipMap.computeIfAbsent(upper, k -> new VipIndexSupport()).addInstance(info); + return; + } + + // Multiple VIPs: uppercase once, then parse without split() byte[] allocation + int start = 0; + do { + String vipAddress = upper.substring(start, commaIndex); + vipMap.computeIfAbsent(vipAddress, k -> new VipIndexSupport()).addInstance(info); + start = commaIndex + 1; + } while ((commaIndex = upper.indexOf(',', start)) != -1); + + // Last segment + String vipAddress = upper.substring(start); + vipMap.computeIfAbsent(vipAddress, k -> new VipIndexSupport()).addInstance(info); } /** From 44b154e6e220c2c0d5ec21317b80a07942a19df5 Mon Sep 17 00:00:00 2001 From: Jason Koch Date: Fri, 9 Jan 2026 09:43:09 -0800 Subject: [PATCH 09/15] perf: optimise shuffleInstances allocations in Applications Specific changes to avoid needing to shuffle and allocate in the zero or one instance case. --- .../discovery/shared/Applications.java | 49 +++++++++++++++---- 1 file changed, 39 insertions(+), 10 deletions(-) diff --git a/eureka-client/src/main/java/com/netflix/discovery/shared/Applications.java b/eureka-client/src/main/java/com/netflix/discovery/shared/Applications.java index bdc8031dcf..f885eb58f9 100644 --- a/eureka-client/src/main/java/com/netflix/discovery/shared/Applications.java +++ b/eureka-client/src/main/java/com/netflix/discovery/shared/Applications.java @@ -396,22 +396,51 @@ public AtomicLong getNextIndex(String virtualHostname, boolean secure) { * */ private void shuffleAndFilterInstances(Map srcMap, boolean filterUpInstances) { - Random shuffleRandom = new Random(); for (Map.Entry entries : srcMap.entrySet()) { - VipIndexSupport vipIndexSupport = entries.getValue(); - List vipInstances = vipIndexSupport.getInstances(); - final List filteredInstances; + shuffleAndFilterInstances(entries.getValue(), filterUpInstances, shuffleRandom); + } + } + + /** + * Shuffle and filter instances for a single VIP. + */ + private void shuffleAndFilterInstances(VipIndexSupport vipIndexSupport, boolean filterUpInstances, Random shuffleRandom) { + // Statistics from prod: 56% of VIPs have exactly 1 instance, 82% have <=3 instances. + // Optimized to avoid allocations for 0/1 instance cases. + List vipInstances = vipIndexSupport.getInstances(); + int size = vipInstances.size(); + + final List filteredInstances; + if (size == 0) { + // Empty: reuse existing emptyList + filteredInstances = vipInstances; + } else if (size == 1) { + // Single instance (56% of VIPs): no shuffle needed, reuse list if possible + InstanceInfo instance = vipInstances.get(0); + if (filterUpInstances && instance.getStatus() != InstanceStatus.UP) { + filteredInstances = Collections.emptyList(); + } else { + filteredInstances = vipInstances; + } + } else { + // Multiple instances: filter with loop (avoids stream allocations) and shuffle + ArrayList list = new ArrayList<>(size); if (filterUpInstances) { - filteredInstances = vipInstances.stream().filter(ii -> ii.getStatus() == InstanceStatus.UP) - .collect(Collectors.toCollection(() -> new ArrayList<>(vipInstances.size()))); + for (int i = 0; i < size; i++) { + InstanceInfo instance = vipInstances.get(i); + if (instance.getStatus() == InstanceStatus.UP) { + list.add(instance); + } + } } else { - filteredInstances = new ArrayList(vipInstances); + list.addAll(vipInstances); } - Collections.shuffle(filteredInstances, shuffleRandom); - vipIndexSupport.vipList.set(filteredInstances); - vipIndexSupport.roundRobinIndex.set(0); + Collections.shuffle(list, shuffleRandom); + filteredInstances = list; } + vipIndexSupport.vipList.set(filteredInstances); + vipIndexSupport.roundRobinIndex.set(0); } /** From d21de38d01ae2546db6c9c3caf074373ce221537 Mon Sep 17 00:00:00 2001 From: Jason Koch Date: Thu, 26 Feb 2026 11:50:27 -0800 Subject: [PATCH 10/15] perf: presize hashmaps in Applications shuffleInstances Squashed from 2af7d67f and 3eaf5aab to avoid transient guava dependency. Uses custom MapUtil.newHashMapWithExpectedSize instead of guava Maps. --- .../discovery/shared/Applications.java | 7 +-- .../com/netflix/discovery/util/MapUtil.java | 43 +++++++++++++++++++ 2 files changed, 47 insertions(+), 3 deletions(-) create mode 100644 eureka-client/src/main/java/com/netflix/discovery/util/MapUtil.java diff --git a/eureka-client/src/main/java/com/netflix/discovery/shared/Applications.java b/eureka-client/src/main/java/com/netflix/discovery/shared/Applications.java index f885eb58f9..9d55999e34 100644 --- a/eureka-client/src/main/java/com/netflix/discovery/shared/Applications.java +++ b/eureka-client/src/main/java/com/netflix/discovery/shared/Applications.java @@ -20,7 +20,6 @@ import java.util.AbstractQueue; import java.util.ArrayList; import java.util.Collections; -import java.util.HashMap; import java.util.List; import java.util.Locale; import java.util.Map; @@ -35,6 +34,8 @@ import java.util.function.Consumer; import java.util.stream.Collectors; +import com.netflix.discovery.util.MapUtil; + import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonIgnore; import com.fasterxml.jackson.annotation.JsonProperty; @@ -353,8 +354,8 @@ private void shuffleInstances(boolean filterUpInstances, @Nullable Map remoteRegionsRegistry, @Nullable EurekaClientConfig clientConfig, @Nullable InstanceRegionChecker instanceRegionChecker) { - Map secureVirtualHostNameAppMap = new HashMap<>(); - Map virtualHostNameAppMap = new HashMap<>(); + Map secureVirtualHostNameAppMap = MapUtil.newHashMapWithExpectedSize(this.secureVirtualHostNameAppMap.size()); + Map virtualHostNameAppMap = MapUtil.newHashMapWithExpectedSize(this.virtualHostNameAppMap.size()); for (Application application : appNameApplicationMap.values()) { if (indexByRemoteRegions) { application.shuffleAndStoreInstances(remoteRegionsRegistry, clientConfig, instanceRegionChecker); diff --git a/eureka-client/src/main/java/com/netflix/discovery/util/MapUtil.java b/eureka-client/src/main/java/com/netflix/discovery/util/MapUtil.java new file mode 100644 index 0000000000..182cdc28e7 --- /dev/null +++ b/eureka-client/src/main/java/com/netflix/discovery/util/MapUtil.java @@ -0,0 +1,43 @@ +/* + * Copyright 2026 Netflix, Inc. + * + * Licensed 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 com.netflix.discovery.util; + +import java.util.HashMap; + +/** + * Utility methods for working with Maps. + */ +public final class MapUtil { + + private MapUtil() { + // Utility class + } + + /** + * Creates a HashMap with enough capacity to hold the expected number of entries + * without resizing. This is equivalent to Guava's Maps.newHashMapWithExpectedSize(). + * + * @param expectedSize the expected number of entries + * @param the key type + * @param the value type + * @return a new HashMap with appropriate initial capacity + */ + public static HashMap newHashMapWithExpectedSize(int expectedSize) { + // HashMap resizes at load factor 0.75, so we need capacity = expectedSize / 0.75 + 1 + return new HashMap<>((int) (expectedSize / 0.75f) + 1); + } +} From cf4a29a0b32929700564454690a7a904235ccbed Mon Sep 17 00:00:00 2001 From: Jason Koch Date: Fri, 9 Jan 2026 18:15:00 -0800 Subject: [PATCH 11/15] perf: swap AtomicRef to volatile in VipIndexSupport --- .../netflix/discovery/shared/Applications.java | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/eureka-client/src/main/java/com/netflix/discovery/shared/Applications.java b/eureka-client/src/main/java/com/netflix/discovery/shared/Applications.java index 9d55999e34..11c12af655 100644 --- a/eureka-client/src/main/java/com/netflix/discovery/shared/Applications.java +++ b/eureka-client/src/main/java/com/netflix/discovery/shared/Applications.java @@ -30,7 +30,6 @@ import java.util.concurrent.ConcurrentLinkedQueue; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicLong; -import java.util.concurrent.atomic.AtomicReference; import java.util.function.Consumer; import java.util.stream.Collectors; @@ -71,7 +70,7 @@ private static class VipIndexSupport { // This avoids CLQ and Node allocations. 56% of VIPs have exactly 1 instance. private List instances = Collections.emptyList(); final AtomicLong roundRobinIndex = new AtomicLong(0); - final AtomicReference> vipList = new AtomicReference<>(Collections.emptyList()); + private volatile List vipList = Collections.emptyList(); void addInstance(InstanceInfo info) { int size = instances.size(); @@ -103,9 +102,13 @@ public AtomicLong getRoundRobinIndex() { return roundRobinIndex; } - public AtomicReference> getVipList() { + List getVipList() { return vipList; } + + void setVipList(List vipList) { + this.vipList = vipList; + } } private static final String STATUS_DELIMITER = "_"; @@ -202,8 +205,7 @@ public Application getRegisteredApplications(String appName) { public List getInstancesByVirtualHostName(String virtualHostName) { return Optional.ofNullable(this.virtualHostNameAppMap.get(virtualHostName.toUpperCase(Locale.ROOT))) .map(VipIndexSupport::getVipList) - .map(AtomicReference::get) - .orElseGet(Collections::emptyList); + .orElseGet(Collections::emptyList); } /** @@ -218,8 +220,7 @@ public List getInstancesByVirtualHostName(String virtualHostName) public List getInstancesBySecureVirtualHostName(String secureVirtualHostName) { return Optional.ofNullable(this.secureVirtualHostNameAppMap.get(secureVirtualHostName.toUpperCase(Locale.ROOT))) .map(VipIndexSupport::getVipList) - .map(AtomicReference::get) - .orElseGet(Collections::emptyList); + .orElseGet(Collections::emptyList); } /** @@ -440,7 +441,7 @@ private void shuffleAndFilterInstances(VipIndexSupport vipIndexSupport, boolean Collections.shuffle(list, shuffleRandom); filteredInstances = list; } - vipIndexSupport.vipList.set(filteredInstances); + vipIndexSupport.setVipList(filteredInstances); vipIndexSupport.roundRobinIndex.set(0); } From 1162833e0e906b11ae6e5c8209bd0d4006b3200b Mon Sep 17 00:00:00 2001 From: Jason Koch Date: Fri, 9 Jan 2026 22:39:31 -0800 Subject: [PATCH 12/15] perf: reduce allocations in Applications shuffleAndFilterInstances --- .../discovery/shared/Applications.java | 80 ++++++++++++------- 1 file changed, 51 insertions(+), 29 deletions(-) diff --git a/eureka-client/src/main/java/com/netflix/discovery/shared/Applications.java b/eureka-client/src/main/java/com/netflix/discovery/shared/Applications.java index 11c12af655..0d8ad00d21 100644 --- a/eureka-client/src/main/java/com/netflix/discovery/shared/Applications.java +++ b/eureka-client/src/main/java/com/netflix/discovery/shared/Applications.java @@ -408,41 +408,63 @@ private void shuffleAndFilterInstances(Map srcMap, bool * Shuffle and filter instances for a single VIP. */ private void shuffleAndFilterInstances(VipIndexSupport vipIndexSupport, boolean filterUpInstances, Random shuffleRandom) { - // Statistics from prod: 56% of VIPs have exactly 1 instance, 82% have <=3 instances. - // Optimized to avoid allocations for 0/1 instance cases. - List vipInstances = vipIndexSupport.getInstances(); - int size = vipInstances.size(); + List instances = vipIndexSupport.getInstances(); + int size = instances.size(); - final List filteredInstances; + // Empty: nothing to do if (size == 0) { - // Empty: reuse existing emptyList - filteredInstances = vipInstances; - } else if (size == 1) { - // Single instance (56% of VIPs): no shuffle needed, reuse list if possible - InstanceInfo instance = vipInstances.get(0); - if (filterUpInstances && instance.getStatus() != InstanceStatus.UP) { - filteredInstances = Collections.emptyList(); - } else { - filteredInstances = vipInstances; + vipIndexSupport.setVipList(instances); + return; + } + + // Single instance: no shuffle needed, check status if filtering + if (size == 1) { + InstanceInfo instance = instances.get(0); + boolean keep = !filterUpInstances || instance.getStatus() == InstanceStatus.UP; + vipIndexSupport.setVipList(keep ? instances : Collections.emptyList()); + return; + } + + // Multiple instances (2+): instances is always an ArrayList at this point + ArrayList list = (ArrayList) instances; + + // Filter in place if needed (no-op when all instances are UP) + if (filterUpInstances) { + filterToUpInstancesInPlace(list); + if (list.isEmpty()) { + vipIndexSupport.setVipList(Collections.emptyList()); + return; } - } else { - // Multiple instances: filter with loop (avoids stream allocations) and shuffle - ArrayList list = new ArrayList<>(size); - if (filterUpInstances) { - for (int i = 0; i < size; i++) { - InstanceInfo instance = vipInstances.get(i); - if (instance.getStatus() == InstanceStatus.UP) { - list.add(instance); - } + } + + // Shuffle in place and reuse + Collections.shuffle(list, shuffleRandom); + vipIndexSupport.setVipList(list); + } + + /** + * Filter list in place to keep only UP instances. Allocation-free. + */ + private static void filterToUpInstancesInPlace(ArrayList list) { + int size = list.size(); + int writeIndex = 0; + // shift forward all of the UP instances + for (int i = 0; i < size; i++) { + InstanceInfo instance = list.get(i); + if (instance.getStatus() == InstanceStatus.UP) { + if (writeIndex != i) { + list.set(writeIndex, instance); } - } else { - list.addAll(vipInstances); + writeIndex++; } - Collections.shuffle(list, shuffleRandom); - filteredInstances = list; } - vipIndexSupport.setVipList(filteredInstances); - vipIndexSupport.roundRobinIndex.set(0); + // Truncate: remove tail elements. Allows old objects to be GCd. + // Array is not shrunk back, but, in the majority case this is not useful. + // More important that we clear the entries so the InstanceInfo elements + // can be released. + if (writeIndex < size) { + list.subList(writeIndex, size).clear(); + } } /** From 680726fcd2c895ad4424304c4f925f6228a055cb Mon Sep 17 00:00:00 2001 From: Jason Koch Date: Fri, 16 Jan 2026 12:59:25 -0800 Subject: [PATCH 13/15] feedback: add explanation for choice of 12 for ArrayMap sizing --- .../java/com/netflix/discovery/shared/Applications.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/eureka-client/src/main/java/com/netflix/discovery/shared/Applications.java b/eureka-client/src/main/java/com/netflix/discovery/shared/Applications.java index 0d8ad00d21..bdc1d3cbb9 100644 --- a/eureka-client/src/main/java/com/netflix/discovery/shared/Applications.java +++ b/eureka-client/src/main/java/com/netflix/discovery/shared/Applications.java @@ -78,7 +78,11 @@ void addInstance(InstanceInfo info) { // 0 -> 1: use singletonList (56% of VIPs stop here) instances = Collections.singletonList(info); } else if (size == 1) { - // 1 -> 2: transition singletonList to ArrayList + // 1 -> 2: transition singletonList to ArrayList. + // Capacity 12 chosen based on prod data analysis: covers 81% of multi-instance + // VIPs without resize (spikes at 6, 9, 12 instances from 3-AZ deployments). + // ArrayList grows 1.5x (12->18->27), aligning well with common sizes. + // Capacity 12 minimizes total allocation vs smaller capacities. InstanceInfo first = instances.get(0); ArrayList list = new ArrayList<>(12); list.add(first); From f54320c6d5f2c7ccd8a202f51305b824d454e5e2 Mon Sep 17 00:00:00 2001 From: Jason Koch Date: Fri, 16 Jan 2026 13:01:00 -0800 Subject: [PATCH 14/15] feedback: add comprehensive test for Applications refactors --- .../discovery/shared/ApplicationsTest.java | 654 +++++++++++++++++- 1 file changed, 653 insertions(+), 1 deletion(-) diff --git a/eureka-client/src/test/java/com/netflix/discovery/shared/ApplicationsTest.java b/eureka-client/src/test/java/com/netflix/discovery/shared/ApplicationsTest.java index a1f30c184e..3b0c1b6489 100644 --- a/eureka-client/src/test/java/com/netflix/discovery/shared/ApplicationsTest.java +++ b/eureka-client/src/test/java/com/netflix/discovery/shared/ApplicationsTest.java @@ -389,6 +389,658 @@ public DataCenterInfo.Name getName() { assertNotNull(applications.getRegisteredApplications("TestApp").getByInstanceId("test.hostname")); assertTrue(applications.getInstancesBySecureVirtualHostName("securetest.testname:7102").isEmpty()); assertTrue(applications.getInstancesBySecureVirtualHostName("test.testname:1").isEmpty()); - } + } + + // ==================== VIP Address Parsing Tests ==================== + + private static final DataCenterInfo TEST_DCI = () -> DataCenterInfo.Name.MyOwn; + + @Test + public void testVipParsing_nullVip() { + Application app = new Application("TestApp"); + Applications apps = new Applications(); + apps.addApplication(app); + apps.shuffleInstances(false); + + // No instances added + assertEquals(0, apps.getRegisteredApplications("TestApp").size()); + assertEquals(0, apps.getRegisteredApplications("TestApp").getInstancesAsIsFromEureka().size()); + // No VIP mappings exist + assertTrue(apps.getInstancesByVirtualHostName("anything").isEmpty()); + } + + @Test + public void testVipParsing_emptyVip() { + InstanceInfo instance = InstanceInfo.Builder.newBuilder() + .setAppName("test").setVIPAddress("").setDataCenterInfo(TEST_DCI) + .setHostName("host1").setStatus(InstanceStatus.UP).build(); + Application app = new Application("TestApp"); + app.addInstance(instance); + Applications apps = new Applications(); + apps.addApplication(app); + apps.shuffleInstances(false); + + // Instance exists in application but no VIP mapping + assertEquals(1, apps.getRegisteredApplications("TestApp").size()); + assertEquals(1, apps.getRegisteredApplications("TestApp").getInstancesAsIsFromEureka().size()); + assertTrue(apps.getInstancesByVirtualHostName("").isEmpty()); + } + + @Test + public void testVipParsing_singleVip() { + InstanceInfo instance = InstanceInfo.Builder.newBuilder() + .setAppName("test").setVIPAddress("my.vip").setDataCenterInfo(TEST_DCI) + .setHostName("host1").setStatus(InstanceStatus.UP).build(); + Application app = new Application("TestApp"); + app.addInstance(instance); + Applications apps = new Applications(); + apps.addApplication(app); + apps.shuffleInstances(false); + + assertEquals(1, apps.getRegisteredApplications("TestApp").size()); + assertEquals(1, apps.getRegisteredApplications("TestApp").getInstancesAsIsFromEureka().size()); + assertEquals(1, apps.getInstancesByVirtualHostName("my.vip").size()); + assertEquals(1, apps.getInstancesByVirtualHostName("MY.VIP").size()); // case-insensitive lookup + assertTrue(apps.getInstancesByVirtualHostName("other.vip").isEmpty()); // no other entries + } + + @Test + public void testVipParsing_singleVipMixedCase() { + InstanceInfo instance = InstanceInfo.Builder.newBuilder() + .setAppName("test").setVIPAddress("My.Vip.Address").setDataCenterInfo(TEST_DCI) + .setHostName("host1").setStatus(InstanceStatus.UP).build(); + Application app = new Application("TestApp"); + app.addInstance(instance); + Applications apps = new Applications(); + apps.addApplication(app); + apps.shuffleInstances(false); + + assertEquals(1, apps.getRegisteredApplications("TestApp").size()); + assertEquals(1, apps.getRegisteredApplications("TestApp").getInstancesAsIsFromEureka().size()); + // All case variations should resolve to same entry + assertEquals(1, apps.getInstancesByVirtualHostName("my.vip.address").size()); + assertEquals(1, apps.getInstancesByVirtualHostName("MY.VIP.ADDRESS").size()); + assertEquals(1, apps.getInstancesByVirtualHostName("My.Vip.Address").size()); + } + + @Test + public void testVipParsing_twoVips() { + InstanceInfo instance = InstanceInfo.Builder.newBuilder() + .setAppName("test").setVIPAddress("vip1,vip2").setDataCenterInfo(TEST_DCI) + .setHostName("host1").setStatus(InstanceStatus.UP).build(); + Application app = new Application("TestApp"); + app.addInstance(instance); + Applications apps = new Applications(); + apps.addApplication(app); + apps.shuffleInstances(false); + + // One instance in the application + assertEquals(1, apps.getRegisteredApplications("TestApp").size()); + assertEquals(1, apps.getRegisteredApplications("TestApp").getInstancesAsIsFromEureka().size()); + // Mapped to two VIPs + assertEquals(1, apps.getInstancesByVirtualHostName("vip1").size()); + assertEquals(1, apps.getInstancesByVirtualHostName("vip2").size()); + // No spurious entries + assertTrue(apps.getInstancesByVirtualHostName("vip1,vip2").isEmpty()); + assertTrue(apps.getInstancesByVirtualHostName("vip3").isEmpty()); + } + + @Test + public void testVipParsing_threeVips() { + InstanceInfo instance = InstanceInfo.Builder.newBuilder() + .setAppName("test").setVIPAddress("vip1,vip2,vip3").setDataCenterInfo(TEST_DCI) + .setHostName("host1").setStatus(InstanceStatus.UP).build(); + Application app = new Application("TestApp"); + app.addInstance(instance); + Applications apps = new Applications(); + apps.addApplication(app); + apps.shuffleInstances(false); + + assertEquals(1, apps.getRegisteredApplications("TestApp").size()); + assertEquals(1, apps.getRegisteredApplications("TestApp").getInstancesAsIsFromEureka().size()); + assertEquals(1, apps.getInstancesByVirtualHostName("vip1").size()); + assertEquals(1, apps.getInstancesByVirtualHostName("vip2").size()); + assertEquals(1, apps.getInstancesByVirtualHostName("vip3").size()); + // No spurious entries from parsing + assertTrue(apps.getInstancesByVirtualHostName("vip1,vip2").isEmpty()); + assertTrue(apps.getInstancesByVirtualHostName("vip2,vip3").isEmpty()); + assertTrue(apps.getInstancesByVirtualHostName("vip1,vip2,vip3").isEmpty()); + } + + @Test + public void testVipParsing_multipleVipsMixedCase() { + InstanceInfo instance = InstanceInfo.Builder.newBuilder() + .setAppName("test").setVIPAddress("Vip.One,VIP.TWO,vip.three").setDataCenterInfo(TEST_DCI) + .setHostName("host1").setStatus(InstanceStatus.UP).build(); + Application app = new Application("TestApp"); + app.addInstance(instance); + Applications apps = new Applications(); + apps.addApplication(app); + apps.shuffleInstances(false); + + assertEquals(1, apps.getRegisteredApplications("TestApp").size()); + assertEquals(1, apps.getRegisteredApplications("TestApp").getInstancesAsIsFromEureka().size()); + // All stored as uppercase, lookup case-insensitive + assertEquals(1, apps.getInstancesByVirtualHostName("vip.one").size()); + assertEquals(1, apps.getInstancesByVirtualHostName("VIP.ONE").size()); + assertEquals(1, apps.getInstancesByVirtualHostName("vip.two").size()); + assertEquals(1, apps.getInstancesByVirtualHostName("vip.three").size()); + } + + @Test + public void testVipParsing_multipleInstancesOverlappingVips() { + // host1: vip.one, vip.two, vip.three + // host2: vip.two, vip.four + // host3: vip.three, vip.four, vip.five + InstanceInfo host1 = InstanceInfo.Builder.newBuilder() + .setAppName("test").setVIPAddress("vip.one,vip.two,vip.three").setDataCenterInfo(TEST_DCI) + .setHostName("host1").setStatus(InstanceStatus.UP).build(); + InstanceInfo host2 = InstanceInfo.Builder.newBuilder() + .setAppName("test").setVIPAddress("vip.two,vip.four").setDataCenterInfo(TEST_DCI) + .setHostName("host2").setStatus(InstanceStatus.UP).build(); + InstanceInfo host3 = InstanceInfo.Builder.newBuilder() + .setAppName("test").setVIPAddress("vip.three,vip.four,vip.five").setDataCenterInfo(TEST_DCI) + .setHostName("host3").setStatus(InstanceStatus.UP).build(); + + Application app = new Application("TestApp"); + app.addInstance(host1); + app.addInstance(host2); + app.addInstance(host3); + Applications apps = new Applications(); + apps.addApplication(app); + apps.shuffleInstances(false); + + // Verify application contains all 3 instances + assertEquals(3, apps.getRegisteredApplications("TestApp").size()); + assertEquals(3, apps.getRegisteredApplications("TestApp").getInstancesAsIsFromEureka().size()); + + // vip.one -> host1 only + List vipOneInstances = apps.getInstancesByVirtualHostName("vip.one"); + assertEquals(1, vipOneInstances.size()); + assertEquals("host1", vipOneInstances.get(0).getHostName()); + + // vip.two -> host1, host2 + List vipTwoInstances = apps.getInstancesByVirtualHostName("vip.two"); + assertEquals(2, vipTwoInstances.size()); + assertTrue(vipTwoInstances.stream().anyMatch(i -> "host1".equals(i.getHostName()))); + assertTrue(vipTwoInstances.stream().anyMatch(i -> "host2".equals(i.getHostName()))); + + // vip.three -> host1, host3 + List vipThreeInstances = apps.getInstancesByVirtualHostName("vip.three"); + assertEquals(2, vipThreeInstances.size()); + assertTrue(vipThreeInstances.stream().anyMatch(i -> "host1".equals(i.getHostName()))); + assertTrue(vipThreeInstances.stream().anyMatch(i -> "host3".equals(i.getHostName()))); + + // vip.four -> host2, host3 + List vipFourInstances = apps.getInstancesByVirtualHostName("vip.four"); + assertEquals(2, vipFourInstances.size()); + assertTrue(vipFourInstances.stream().anyMatch(i -> "host2".equals(i.getHostName()))); + assertTrue(vipFourInstances.stream().anyMatch(i -> "host3".equals(i.getHostName()))); + + // vip.five -> host3 only + List vipFiveInstances = apps.getInstancesByVirtualHostName("vip.five"); + assertEquals(1, vipFiveInstances.size()); + assertEquals("host3", vipFiveInstances.get(0).getHostName()); + + // Case-insensitive lookups work + assertEquals(2, apps.getInstancesByVirtualHostName("VIP.TWO").size()); + assertEquals(2, apps.getInstancesByVirtualHostName("VIP.FOUR").size()); + + // No spurious VIP entries + assertTrue(apps.getInstancesByVirtualHostName("vip.six").isEmpty()); + assertTrue(apps.getInstancesByVirtualHostName("vip.one,vip.two").isEmpty()); + } + + // ==================== shuffleAndFilterInstances Tests ==================== + + @Test + public void testMultipleInstancesFiltering() { + DataCenterInfo myDCI = new DataCenterInfo() { + public DataCenterInfo.Name getName() { + return DataCenterInfo.Name.MyOwn; + } + }; + InstanceInfo up1 = InstanceInfo.Builder.newBuilder().setAppName("test") + .setVIPAddress("test.vip").setDataCenterInfo(myDCI) + .setHostName("up1.hostname").setStatus(InstanceStatus.UP).build(); + InstanceInfo up2 = InstanceInfo.Builder.newBuilder().setAppName("test") + .setVIPAddress("test.vip").setDataCenterInfo(myDCI) + .setHostName("up2.hostname").setStatus(InstanceStatus.UP).build(); + InstanceInfo down = InstanceInfo.Builder.newBuilder().setAppName("test") + .setVIPAddress("test.vip").setDataCenterInfo(myDCI) + .setHostName("down.hostname").setStatus(InstanceStatus.DOWN).build(); + + Application application = new Application("TestApp"); + application.addInstance(up1); + application.addInstance(up2); + application.addInstance(down); + + Applications applications = new Applications(); + applications.addApplication(application); + applications.shuffleInstances(true); + + List result = applications.getInstancesByVirtualHostName("test.vip"); + assertEquals(2, result.size()); + assertTrue(result.contains(up1)); + assertTrue(result.contains(up2)); + assertFalse(result.contains(down)); + } + + @Test + public void testSingleInstanceUp_filterEnabled() { + InstanceInfo instance = InstanceInfo.Builder.newBuilder() + .setAppName("test").setVIPAddress("my.vip").setDataCenterInfo(TEST_DCI) + .setHostName("host1").setStatus(InstanceStatus.UP).build(); + Application app = new Application("TestApp"); + app.addInstance(instance); + Applications apps = new Applications(); + apps.addApplication(app); + apps.shuffleInstances(true); + + List result = apps.getInstancesByVirtualHostName("my.vip"); + assertEquals(1, result.size()); + assertTrue(result.contains(instance)); + } + + @Test + public void testSingleInstanceDown_filterEnabled() { + InstanceInfo instance = InstanceInfo.Builder.newBuilder() + .setAppName("test").setVIPAddress("my.vip").setDataCenterInfo(TEST_DCI) + .setHostName("host1").setStatus(InstanceStatus.DOWN).build(); + Application app = new Application("TestApp"); + app.addInstance(instance); + Applications apps = new Applications(); + apps.addApplication(app); + apps.shuffleInstances(true); + + // Single DOWN instance should be filtered out + List result = apps.getInstancesByVirtualHostName("my.vip"); + assertTrue(result.isEmpty()); + // But instance still exists in the application + assertEquals(1, apps.getRegisteredApplications("TestApp").size()); + } + + @Test + public void testMultipleInstances_filterDisabled() { + InstanceInfo up = InstanceInfo.Builder.newBuilder() + .setAppName("test").setVIPAddress("my.vip").setDataCenterInfo(TEST_DCI) + .setHostName("up.host").setStatus(InstanceStatus.UP).build(); + InstanceInfo down = InstanceInfo.Builder.newBuilder() + .setAppName("test").setVIPAddress("my.vip").setDataCenterInfo(TEST_DCI) + .setHostName("down.host").setStatus(InstanceStatus.DOWN).build(); + InstanceInfo oos = InstanceInfo.Builder.newBuilder() + .setAppName("test").setVIPAddress("my.vip").setDataCenterInfo(TEST_DCI) + .setHostName("oos.host").setStatus(InstanceStatus.OUT_OF_SERVICE).build(); + + Application app = new Application("TestApp"); + app.addInstance(up); + app.addInstance(down); + app.addInstance(oos); + Applications apps = new Applications(); + apps.addApplication(app); + apps.shuffleInstances(false); // filterUpInstances = false + + // All instances should be present regardless of status + List result = apps.getInstancesByVirtualHostName("my.vip"); + assertEquals(3, result.size()); + assertTrue(result.contains(up)); + assertTrue(result.contains(down)); + assertTrue(result.contains(oos)); + } + + @Test + public void testAllInstancesNonUp_filterEnabled() { + InstanceInfo down = InstanceInfo.Builder.newBuilder() + .setAppName("test").setVIPAddress("my.vip").setDataCenterInfo(TEST_DCI) + .setHostName("down.host").setStatus(InstanceStatus.DOWN).build(); + InstanceInfo oos = InstanceInfo.Builder.newBuilder() + .setAppName("test").setVIPAddress("my.vip").setDataCenterInfo(TEST_DCI) + .setHostName("oos.host").setStatus(InstanceStatus.OUT_OF_SERVICE).build(); + InstanceInfo starting = InstanceInfo.Builder.newBuilder() + .setAppName("test").setVIPAddress("my.vip").setDataCenterInfo(TEST_DCI) + .setHostName("starting.host").setStatus(InstanceStatus.STARTING).build(); + + Application app = new Application("TestApp"); + app.addInstance(down); + app.addInstance(oos); + app.addInstance(starting); + Applications apps = new Applications(); + apps.addApplication(app); + apps.shuffleInstances(true); + + // All instances filtered out + List result = apps.getInstancesByVirtualHostName("my.vip"); + assertTrue(result.isEmpty()); + // But all instances still exist in the application + assertEquals(3, apps.getRegisteredApplications("TestApp").size()); + } + + @Test + public void testSecureVipFiltering() { + InstanceInfo up = InstanceInfo.Builder.newBuilder() + .setAppName("test").setSecureVIPAddress("secure.vip").setDataCenterInfo(TEST_DCI) + .setHostName("up.host").setStatus(InstanceStatus.UP).build(); + InstanceInfo down = InstanceInfo.Builder.newBuilder() + .setAppName("test").setSecureVIPAddress("secure.vip").setDataCenterInfo(TEST_DCI) + .setHostName("down.host").setStatus(InstanceStatus.DOWN).build(); + + Application app = new Application("TestApp"); + app.addInstance(up); + app.addInstance(down); + Applications apps = new Applications(); + apps.addApplication(app); + apps.shuffleInstances(true); + + List result = apps.getInstancesBySecureVirtualHostName("secure.vip"); + assertEquals(1, result.size()); + assertTrue(result.contains(up)); + assertFalse(result.contains(down)); + } + + @Test + public void testReshuffleUpdatesFilteredList() { + InstanceInfo host1 = InstanceInfo.Builder.newBuilder() + .setAppName("test").setVIPAddress("my.vip").setDataCenterInfo(TEST_DCI) + .setHostName("host1").setStatus(InstanceStatus.UP).build(); + InstanceInfo host2 = InstanceInfo.Builder.newBuilder() + .setAppName("test").setVIPAddress("my.vip").setDataCenterInfo(TEST_DCI) + .setHostName("host2").setStatus(InstanceStatus.UP).build(); + + Application app = new Application("TestApp"); + app.addInstance(host1); + app.addInstance(host2); + Applications apps = new Applications(); + apps.addApplication(app); + apps.shuffleInstances(true); + + assertEquals(2, apps.getInstancesByVirtualHostName("my.vip").size()); + + // Change host2 to DOWN and reshuffle + host2.setStatus(InstanceStatus.DOWN); + apps.shuffleInstances(true); + + // Only host1 should remain + List result = apps.getInstancesByVirtualHostName("my.vip"); + assertEquals(1, result.size()); + assertTrue(result.contains(host1)); + assertFalse(result.contains(host2)); + } + + @Test + public void testMixedVipAndSecureVipFiltering() { + InstanceInfo upBoth = InstanceInfo.Builder.newBuilder() + .setAppName("test").setVIPAddress("my.vip").setSecureVIPAddress("secure.vip") + .setDataCenterInfo(TEST_DCI).setHostName("up.both").setStatus(InstanceStatus.UP).build(); + InstanceInfo downBoth = InstanceInfo.Builder.newBuilder() + .setAppName("test").setVIPAddress("my.vip").setSecureVIPAddress("secure.vip") + .setDataCenterInfo(TEST_DCI).setHostName("down.both").setStatus(InstanceStatus.DOWN).build(); + InstanceInfo upVipOnly = InstanceInfo.Builder.newBuilder() + .setAppName("test").setVIPAddress("my.vip") + .setDataCenterInfo(TEST_DCI).setHostName("up.vip").setStatus(InstanceStatus.UP).build(); + + Application app = new Application("TestApp"); + app.addInstance(upBoth); + app.addInstance(downBoth); + app.addInstance(upVipOnly); + Applications apps = new Applications(); + apps.addApplication(app); + apps.shuffleInstances(true); + + // VIP should have 2 UP instances + List vipResult = apps.getInstancesByVirtualHostName("my.vip"); + assertEquals(2, vipResult.size()); + assertTrue(vipResult.contains(upBoth)); + assertTrue(vipResult.contains(upVipOnly)); + + // Secure VIP should have 1 UP instance + List secureResult = apps.getInstancesBySecureVirtualHostName("secure.vip"); + assertEquals(1, secureResult.size()); + assertTrue(secureResult.contains(upBoth)); + } + + // ==================== Collection Progression Tests (emptyList -> singletonList -> ArrayList) ==================== + + @Test + public void testVipInstanceCountProgression() { + // Explicitly test 0->1->2->3->4 instance progression on same VIP + // Validates emptyList -> singletonList -> ArrayList(12) transitions + InstanceInfo inst1 = InstanceInfo.Builder.newBuilder() + .setAppName("test").setVIPAddress("my.vip").setDataCenterInfo(TEST_DCI) + .setHostName("host1").setStatus(InstanceStatus.UP).build(); + InstanceInfo inst2 = InstanceInfo.Builder.newBuilder() + .setAppName("test").setVIPAddress("my.vip").setDataCenterInfo(TEST_DCI) + .setHostName("host2").setStatus(InstanceStatus.UP).build(); + InstanceInfo inst3 = InstanceInfo.Builder.newBuilder() + .setAppName("test").setVIPAddress("my.vip").setDataCenterInfo(TEST_DCI) + .setHostName("host3").setStatus(InstanceStatus.UP).build(); + InstanceInfo inst4 = InstanceInfo.Builder.newBuilder() + .setAppName("test").setVIPAddress("my.vip").setDataCenterInfo(TEST_DCI) + .setHostName("host4").setStatus(InstanceStatus.UP).build(); + + Application app = new Application("TestApp"); + Applications apps = new Applications(); + + // 0 instances - no VIP entry exists yet + apps.addApplication(app); + apps.shuffleInstances(false); + assertTrue(apps.getInstancesByVirtualHostName("my.vip").isEmpty()); + + // 1 instance - singletonList path + app.addInstance(inst1); + apps = new Applications(); + apps.addApplication(app); + apps.shuffleInstances(false); + List result1 = apps.getInstancesByVirtualHostName("my.vip"); + assertEquals(1, result1.size()); + assertTrue(result1.contains(inst1)); + + // 2 instances - ArrayList transition + app.addInstance(inst2); + apps = new Applications(); + apps.addApplication(app); + apps.shuffleInstances(false); + List result2 = apps.getInstancesByVirtualHostName("my.vip"); + assertEquals(2, result2.size()); + assertTrue(result2.contains(inst1)); + assertTrue(result2.contains(inst2)); + + // 3 instances - ArrayList grows + app.addInstance(inst3); + apps = new Applications(); + apps.addApplication(app); + apps.shuffleInstances(false); + List result3 = apps.getInstancesByVirtualHostName("my.vip"); + assertEquals(3, result3.size()); + + // 4 instances - ArrayList continues to grow + app.addInstance(inst4); + apps = new Applications(); + apps.addApplication(app); + apps.shuffleInstances(false); + List result4 = apps.getInstancesByVirtualHostName("my.vip"); + assertEquals(4, result4.size()); + assertTrue(result4.contains(inst1)); + assertTrue(result4.contains(inst2)); + assertTrue(result4.contains(inst3)); + assertTrue(result4.contains(inst4)); + } + + @Test + public void testFilteringWithSingletonList_instanceUp() { + // Single UP instance: singletonList should be preserved + InstanceInfo instance = InstanceInfo.Builder.newBuilder() + .setAppName("test").setVIPAddress("my.vip").setDataCenterInfo(TEST_DCI) + .setHostName("host1").setStatus(InstanceStatus.UP).build(); + Application app = new Application("TestApp"); + app.addInstance(instance); + Applications apps = new Applications(); + apps.addApplication(app); + apps.shuffleInstances(true); + + List result = apps.getInstancesByVirtualHostName("my.vip"); + assertEquals(1, result.size()); + assertTrue(result.contains(instance)); + } + + @Test + public void testFilteringWithSingletonList_instanceDown() { + // Single DOWN instance: should return emptyList + InstanceInfo instance = InstanceInfo.Builder.newBuilder() + .setAppName("test").setVIPAddress("my.vip").setDataCenterInfo(TEST_DCI) + .setHostName("host1").setStatus(InstanceStatus.DOWN).build(); + Application app = new Application("TestApp"); + app.addInstance(instance); + Applications apps = new Applications(); + apps.addApplication(app); + apps.shuffleInstances(true); + + List result = apps.getInstancesByVirtualHostName("my.vip"); + assertTrue(result.isEmpty()); + } + + @Test + public void testFilteringWithArrayList_oneUpOneDown() { + // 2 instances (ArrayList), 1 UP 1 DOWN: filters to 1 + InstanceInfo up = InstanceInfo.Builder.newBuilder() + .setAppName("test").setVIPAddress("my.vip").setDataCenterInfo(TEST_DCI) + .setHostName("up.host").setStatus(InstanceStatus.UP).build(); + InstanceInfo down = InstanceInfo.Builder.newBuilder() + .setAppName("test").setVIPAddress("my.vip").setDataCenterInfo(TEST_DCI) + .setHostName("down.host").setStatus(InstanceStatus.DOWN).build(); + Application app = new Application("TestApp"); + app.addInstance(up); + app.addInstance(down); + Applications apps = new Applications(); + apps.addApplication(app); + apps.shuffleInstances(true); + + List result = apps.getInstancesByVirtualHostName("my.vip"); + assertEquals(1, result.size()); + assertTrue(result.contains(up)); + assertFalse(result.contains(down)); + } + + @Test + public void testFilteringWithArrayList_allDown() { + // 3 instances (ArrayList), all DOWN: should return emptyList + InstanceInfo down1 = InstanceInfo.Builder.newBuilder() + .setAppName("test").setVIPAddress("my.vip").setDataCenterInfo(TEST_DCI) + .setHostName("down1.host").setStatus(InstanceStatus.DOWN).build(); + InstanceInfo down2 = InstanceInfo.Builder.newBuilder() + .setAppName("test").setVIPAddress("my.vip").setDataCenterInfo(TEST_DCI) + .setHostName("down2.host").setStatus(InstanceStatus.OUT_OF_SERVICE).build(); + InstanceInfo down3 = InstanceInfo.Builder.newBuilder() + .setAppName("test").setVIPAddress("my.vip").setDataCenterInfo(TEST_DCI) + .setHostName("down3.host").setStatus(InstanceStatus.STARTING).build(); + Application app = new Application("TestApp"); + app.addInstance(down1); + app.addInstance(down2); + app.addInstance(down3); + Applications apps = new Applications(); + apps.addApplication(app); + apps.shuffleInstances(true); + + List result = apps.getInstancesByVirtualHostName("my.vip"); + assertTrue(result.isEmpty()); + } + + @Test + public void testFilteringPreservesUpInstancesInOrder() { + // Verify UP instances are preserved (order may change due to shuffle) + InstanceInfo up1 = InstanceInfo.Builder.newBuilder() + .setAppName("test").setVIPAddress("my.vip").setDataCenterInfo(TEST_DCI) + .setHostName("up1").setStatus(InstanceStatus.UP).build(); + InstanceInfo down1 = InstanceInfo.Builder.newBuilder() + .setAppName("test").setVIPAddress("my.vip").setDataCenterInfo(TEST_DCI) + .setHostName("down1").setStatus(InstanceStatus.DOWN).build(); + InstanceInfo up2 = InstanceInfo.Builder.newBuilder() + .setAppName("test").setVIPAddress("my.vip").setDataCenterInfo(TEST_DCI) + .setHostName("up2").setStatus(InstanceStatus.UP).build(); + InstanceInfo down2 = InstanceInfo.Builder.newBuilder() + .setAppName("test").setVIPAddress("my.vip").setDataCenterInfo(TEST_DCI) + .setHostName("down2").setStatus(InstanceStatus.OUT_OF_SERVICE).build(); + InstanceInfo up3 = InstanceInfo.Builder.newBuilder() + .setAppName("test").setVIPAddress("my.vip").setDataCenterInfo(TEST_DCI) + .setHostName("up3").setStatus(InstanceStatus.UP).build(); + + Application app = new Application("TestApp"); + app.addInstance(up1); + app.addInstance(down1); + app.addInstance(up2); + app.addInstance(down2); + app.addInstance(up3); + Applications apps = new Applications(); + apps.addApplication(app); + apps.shuffleInstances(true); + + List result = apps.getInstancesByVirtualHostName("my.vip"); + assertEquals(3, result.size()); + assertTrue(result.contains(up1)); + assertTrue(result.contains(up2)); + assertTrue(result.contains(up3)); + assertFalse(result.contains(down1)); + assertFalse(result.contains(down2)); + } + + @Test + public void testInstanceRemovalFromApplication() { + // Test that removing an instance from application is reflected after reshuffle + InstanceInfo inst1 = InstanceInfo.Builder.newBuilder() + .setAppName("test").setVIPAddress("my.vip").setDataCenterInfo(TEST_DCI) + .setHostName("host1").setStatus(InstanceStatus.UP).build(); + InstanceInfo inst2 = InstanceInfo.Builder.newBuilder() + .setAppName("test").setVIPAddress("my.vip").setDataCenterInfo(TEST_DCI) + .setHostName("host2").setStatus(InstanceStatus.UP).build(); + InstanceInfo inst3 = InstanceInfo.Builder.newBuilder() + .setAppName("test").setVIPAddress("my.vip").setDataCenterInfo(TEST_DCI) + .setHostName("host3").setStatus(InstanceStatus.UP).build(); + + Application app = new Application("TestApp"); + app.addInstance(inst1); + app.addInstance(inst2); + app.addInstance(inst3); + Applications apps = new Applications(); + apps.addApplication(app); + apps.shuffleInstances(false); + + assertEquals(3, apps.getInstancesByVirtualHostName("my.vip").size()); + + // Remove one instance + app.removeInstance(inst2); + apps.shuffleInstances(false); + + List result = apps.getInstancesByVirtualHostName("my.vip"); + assertEquals(2, result.size()); + assertTrue(result.contains(inst1)); + assertFalse(result.contains(inst2)); + assertTrue(result.contains(inst3)); + } + + @Test + public void testRemoveAllInstancesFromVip() { + // Test removing all instances results in empty VIP list + InstanceInfo inst1 = InstanceInfo.Builder.newBuilder() + .setAppName("test").setVIPAddress("my.vip").setDataCenterInfo(TEST_DCI) + .setHostName("host1").setStatus(InstanceStatus.UP).build(); + InstanceInfo inst2 = InstanceInfo.Builder.newBuilder() + .setAppName("test").setVIPAddress("my.vip").setDataCenterInfo(TEST_DCI) + .setHostName("host2").setStatus(InstanceStatus.UP).build(); + + Application app = new Application("TestApp"); + app.addInstance(inst1); + app.addInstance(inst2); + Applications apps = new Applications(); + apps.addApplication(app); + apps.shuffleInstances(false); + + assertEquals(2, apps.getInstancesByVirtualHostName("my.vip").size()); + + // Remove all instances + app.removeInstance(inst1); + app.removeInstance(inst2); + apps.shuffleInstances(false); + + assertTrue(apps.getInstancesByVirtualHostName("my.vip").isEmpty()); + } } From b8ff51931bd44a4bceed59bc430a168c361165f8 Mon Sep 17 00:00:00 2001 From: Jason Koch Date: Fri, 16 Jan 2026 13:16:00 -0800 Subject: [PATCH 15/15] fix: restore legacy behavior allowing empty vip names --- .../main/java/com/netflix/discovery/shared/Applications.java | 4 +++- .../java/com/netflix/discovery/shared/ApplicationsTest.java | 5 +++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/eureka-client/src/main/java/com/netflix/discovery/shared/Applications.java b/eureka-client/src/main/java/com/netflix/discovery/shared/Applications.java index bdc1d3cbb9..24a232dfa4 100644 --- a/eureka-client/src/main/java/com/netflix/discovery/shared/Applications.java +++ b/eureka-client/src/main/java/com/netflix/discovery/shared/Applications.java @@ -490,7 +490,9 @@ private void addInstanceToMap(InstanceInfo info, String vipAddresses, Map