From 7da6682b64b1626f7b121a0714853348393ef10f Mon Sep 17 00:00:00 2001 From: "A.Fink" Date: Sun, 10 Aug 2025 23:32:23 +0300 Subject: [PATCH 01/10] Headers: toString! fix Nullability; use entrySet instead of keySet+get; fix broken forEach; optimize; missing tests --- .../java/io/nats/client/impl/Headers.java | 81 +++++++++++-------- .../io/nats/client/impl/HeadersTests.java | 42 ++++++++++ 2 files changed, 91 insertions(+), 32 deletions(-) diff --git a/src/main/java/io/nats/client/impl/Headers.java b/src/main/java/io/nats/client/impl/Headers.java index b6af787a1..b07028041 100644 --- a/src/main/java/io/nats/client/impl/Headers.java +++ b/src/main/java/io/nats/client/impl/Headers.java @@ -14,9 +14,11 @@ package io.nats.client.impl; import io.nats.client.support.ByteArrayBuilder; +import org.jspecify.annotations.Nullable; import java.util.*; import java.util.function.BiConsumer; +import java.util.stream.Collectors; import static io.nats.client.support.NatsConstants.*; import static java.nio.charset.StandardCharsets.US_ASCII; @@ -36,8 +38,7 @@ public class Headers { private final Map> valuesMap; private final Map lengthMap; - private final boolean readOnly; - private byte[] serialized; + private byte @Nullable [] serialized; private int dataLength; public Headers() { @@ -52,7 +53,7 @@ public Headers(Headers headers, boolean readOnly) { this(headers, readOnly, null); } - public Headers(Headers headers, boolean readOnly, String[] keysNotToCopy) { + public Headers(@Nullable Headers headers, boolean readOnly, String @Nullable [] keysNotToCopy) { Map> tempValuesMap = new HashMap<>(); Map tempLengthMap = new HashMap<>(); if (headers != null) { @@ -69,7 +70,6 @@ public Headers(Headers headers, boolean readOnly, String[] keysNotToCopy) { } } } - this.readOnly = readOnly; if (readOnly) { valuesMap = Collections.unmodifiableMap(tempValuesMap); lengthMap = Collections.unmodifiableMap(tempLengthMap); @@ -92,7 +92,7 @@ public Headers(Headers headers, boolean readOnly, String[] keysNotToCopy) { * -or- if any value contains invalid characters */ public Headers add(String key, String... values) { - if (readOnly) { + if (isReadOnly()) { throw new UnsupportedOperationException(); } if (values == null || values.length == 0) { @@ -113,7 +113,7 @@ public Headers add(String key, String... values) { * -or- if any value contains invalid characters */ public Headers add(String key, Collection values) { - if (readOnly) { + if (isReadOnly()) { throw new UnsupportedOperationException(); } if (values == null || values.isEmpty()) { @@ -153,7 +153,7 @@ private Headers _add(String key, Collection values) { * -or- if any value contains invalid characters */ public Headers put(String key, String... values) { - if (readOnly) { + if (isReadOnly()) { throw new UnsupportedOperationException(); } if (values == null || values.length == 0) { @@ -174,7 +174,7 @@ public Headers put(String key, String... values) { * -or- if any value contains invalid characters */ public Headers put(String key, Collection values) { - if (readOnly) { + if (isReadOnly()) { throw new UnsupportedOperationException(); } if (values == null || values.isEmpty()) { @@ -191,14 +191,14 @@ public Headers put(String key, Collection values) { * @return the Headers object */ public Headers put(Map> map) { - if (readOnly) { + if (isReadOnly()) { throw new UnsupportedOperationException(); } if (map == null || map.isEmpty()) { return this; } - for (String key : map.keySet() ) { - _put(key, map.get(key)); + for (Map.Entry> entry : map.entrySet()) { + _put(entry.getKey(), entry.getValue()); } return this; } @@ -228,7 +228,7 @@ private Headers _put(String key, Collection values) { * @param keys the key or keys to remove */ public void remove(String... keys) { - if (readOnly) { + if (isReadOnly()) { throw new UnsupportedOperationException(); } for (String key : keys) { @@ -243,7 +243,7 @@ public void remove(String... keys) { * @param keys the key or keys to remove */ public void remove(Collection keys) { - if (readOnly) { + if (isReadOnly()) { throw new UnsupportedOperationException(); } for (String key : keys) { @@ -282,7 +282,7 @@ public boolean isEmpty() { * Removes all the keys The object map will be empty after this call returns. */ public void clear() { - if (readOnly) { + if (isReadOnly()) { throw new UnsupportedOperationException(); } valuesMap.clear(); @@ -331,7 +331,7 @@ public Set keySet() { * @return a read-only set of keys (in lowercase) contained in this map */ public Set keySetIgnoreCase() { - HashSet set = new HashSet<>(); + HashSet set = new HashSet<>(valuesMap.size()*4/3 + 1); for (String k : valuesMap.keySet()) { set.add(k.toLowerCase()); } @@ -345,7 +345,7 @@ public Set keySetIgnoreCase() { * @param key the key whose associated value is to be returned * @return a read-only list of the values for the case-sensitive key. */ - public List get(String key) { + public @Nullable List get(String key) { List values = valuesMap.get(key); return values == null ? null : Collections.unmodifiableList(values); } @@ -356,7 +356,7 @@ public List get(String key) { * @param key the key whose associated value is to be returned * @return the first value for the case-sensitive key. */ - public String getFirst(String key) { + public @Nullable String getFirst(String key) { List values = valuesMap.get(key); return values == null ? null : values.get(0); } @@ -368,7 +368,7 @@ public String getFirst(String key) { * @param key the key whose associated value is to be returned * @return the last value for the case-sensitive key. */ - public String getLast(String key) { + public @Nullable String getLast(String key) { List values = valuesMap.get(key); return values == null ? null : values.get(values.size() - 1); } @@ -380,11 +380,11 @@ public String getLast(String key) { * @param key the key whose associated value is to be returned * @return a read-only list of the values for the case-insensitive key. */ - public List getIgnoreCase(String key) { + public @Nullable List getIgnoreCase(String key) { List values = new ArrayList<>(); - for (String k : valuesMap.keySet()) { - if (k.equalsIgnoreCase(key)) { - values.addAll(valuesMap.get(k)); + for (Map.Entry> entry : valuesMap.entrySet()) { + if (entry.getKey().equalsIgnoreCase(key)) { + values.addAll(entry.getValue()); } } return values.isEmpty() ? null : Collections.unmodifiableList(values); @@ -401,7 +401,8 @@ public List getIgnoreCase(String key) { * removed during iteration */ public void forEach(BiConsumer> action) { - Collections.unmodifiableMap(valuesMap).forEach(action); + valuesMap.forEach((key, values) -> + action.accept(key, Collections.unmodifiableList(values))); } /** @@ -460,9 +461,9 @@ public byte[] getSerialized() { @Deprecated public ByteArrayBuilder appendSerialized(ByteArrayBuilder bab) { bab.append(HEADER_VERSION_BYTES_PLUS_CRLF); - for (String key : valuesMap.keySet()) { - for (String value : valuesMap.get(key)) { - bab.append(key); + for (Map.Entry> entry : valuesMap.entrySet()) { + for (String value : entry.getValue()) { + bab.append(entry.getKey()); bab.append(COLON_BYTES); bab.append(value); bab.append(CRLF_BYTES); @@ -535,11 +536,12 @@ private void checkKey(String key) { private void checkValue(String val) { // Like rfc822 section 3.1.2 (quoted in ADR 4) // The field-body may be composed of any US-ASCII characters, except CR or LF. - val.chars().forEach(c -> { + for (int i = 0, len = val.length(); i < len; i++) { + int c = val.charAt(i); if (c > 127 || c == 10 || c == 13) { - throw new IllegalArgumentException(VALUE_INVALID_CHARACTERS + c); + throw new IllegalArgumentException(VALUE_INVALID_CHARACTERS + Integer.toHexString(c)); } - }); + } } private class Checker { @@ -575,19 +577,34 @@ boolean hasValues() { * @return the read only state */ public boolean isReadOnly() { - return readOnly; + return !(valuesMap instanceof HashMap); } @Override public boolean equals(Object o) { if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; + if (!(o instanceof Headers)) return false; Headers headers = (Headers) o; return Objects.equals(valuesMap, headers.valuesMap); } @Override public int hashCode() { - return Objects.hash(valuesMap); + return Objects.hashCode(valuesMap); + } + + @Override + public String toString() { + return valuesMap.entrySet().stream() + .filter(e -> e.getValue() != null) + .sorted(Map.Entry.comparingByKey()) + .map(e -> { + String headerName = e.getKey(); + List values = e.getValue(); + return headerName +": "+ (values.size() == 1 ? values.get(0) + : String.join(", ", values) + ); + }) + .collect(Collectors.joining("; ")); } } diff --git a/src/test/java/io/nats/client/impl/HeadersTests.java b/src/test/java/io/nats/client/impl/HeadersTests.java index 3e390eacc..8e6da6924 100644 --- a/src/test/java/io/nats/client/impl/HeadersTests.java +++ b/src/test/java/io/nats/client/impl/HeadersTests.java @@ -217,7 +217,13 @@ public void testReadOnly() { assertTrue(headers1.isReadOnly()); assertThrows(UnsupportedOperationException.class, () -> headers1.put(KEY1, VAL2)); assertThrows(UnsupportedOperationException.class, () -> headers1.put(KEY1, VAL2)); + assertThrows(UnsupportedOperationException.class, () -> headers1.put(KEY1, VAL1, VAL2)); + assertThrows(UnsupportedOperationException.class, () -> headers1.put(KEY1, Arrays.asList(VAL1, VAL2))); assertThrows(UnsupportedOperationException.class, () -> headers1.remove(KEY1)); + assertThrows(UnsupportedOperationException.class, () -> headers1.remove(KEY1,KEY2)); + assertThrows(UnsupportedOperationException.class, () -> headers1.remove(Arrays.asList(KEY1,KEY2))); + assertThrows(UnsupportedOperationException.class, () -> headers1.add(KEY1, VAL2)); + assertThrows(UnsupportedOperationException.class, () -> headers1.add(KEY1, Arrays.asList(VAL1, VAL2))); assertThrows(UnsupportedOperationException.class, headers1::clear); assertEquals(VAL1, headers1.getFirst(KEY1)); } @@ -761,6 +767,16 @@ public void testTokenSamePoint() { @Test public void testToString() { assertNotNull(new Status(1, "msg").toString()); // COVERAGE + + Headers h = new Headers(); + h.add("Test1"); + h.add("Test2", "Test2Value"); + h.add("Test3", ""); + h.add("Test4", "", "", ""); + h.add("Test5", "Nice!", "To.", "See?"); + + assertEquals("Test2: Test2Value; Test3: ; Test4: , , ; Test5: Nice!, To., See?", h.toString() + ); } @Test @@ -782,4 +798,30 @@ public void put_map_works() { assertTrue(h.get(KEY2).contains(VAL3)); assertEquals(VAL2, h.getFirst(KEY2)); } + + @Test + void testForEach() { + Headers h = new Headers(); + h.put("test", "a","b","c"); + h.forEach((k, v) -> { + assertEquals("test", k); + assertContainsExactly(v, "a", "b", "c"); + assertThrows(UnsupportedOperationException.class, ()->v.add("z")); + }); + } + + /// @see io.nats.client.impl.Headers#checkValue + @Test + void testCheckValue() { + Headers h = new Headers(); + h.put("test1", "\u0000 \f\b\t"); + + assertThrows(IllegalArgumentException.class, ()->h.put("test", "×")); + assertThrows(IllegalArgumentException.class, ()->h.put("test", "\r")); + assertThrows(IllegalArgumentException.class, ()->h.put("test", "\n")); + + assertEquals(1, h.size()); + assertEquals(1, h.get("test1").size()); + assertEquals("\u0000 \f\b\t", h.getFirst("test1")); + } } From b13a2248c58deca05a9fc021482e2565e7882c99 Mon Sep 17 00:00:00 2001 From: "A.Fink" Date: Tue, 12 Aug 2025 02:47:28 +0300 Subject: [PATCH 02/10] revert readOnly back to field --- .../java/io/nats/client/impl/Headers.java | 20 ++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/src/main/java/io/nats/client/impl/Headers.java b/src/main/java/io/nats/client/impl/Headers.java index b07028041..666af2fb8 100644 --- a/src/main/java/io/nats/client/impl/Headers.java +++ b/src/main/java/io/nats/client/impl/Headers.java @@ -38,6 +38,7 @@ public class Headers { private final Map> valuesMap; private final Map lengthMap; + private final boolean readOnly; private byte @Nullable [] serialized; private int dataLength; @@ -70,6 +71,7 @@ public Headers(@Nullable Headers headers, boolean readOnly, String @Nullable [] } } } + this.readOnly = readOnly; if (readOnly) { valuesMap = Collections.unmodifiableMap(tempValuesMap); lengthMap = Collections.unmodifiableMap(tempLengthMap); @@ -92,7 +94,7 @@ public Headers(@Nullable Headers headers, boolean readOnly, String @Nullable [] * -or- if any value contains invalid characters */ public Headers add(String key, String... values) { - if (isReadOnly()) { + if (readOnly) { throw new UnsupportedOperationException(); } if (values == null || values.length == 0) { @@ -113,7 +115,7 @@ public Headers add(String key, String... values) { * -or- if any value contains invalid characters */ public Headers add(String key, Collection values) { - if (isReadOnly()) { + if (readOnly) { throw new UnsupportedOperationException(); } if (values == null || values.isEmpty()) { @@ -153,7 +155,7 @@ private Headers _add(String key, Collection values) { * -or- if any value contains invalid characters */ public Headers put(String key, String... values) { - if (isReadOnly()) { + if (readOnly) { throw new UnsupportedOperationException(); } if (values == null || values.length == 0) { @@ -174,7 +176,7 @@ public Headers put(String key, String... values) { * -or- if any value contains invalid characters */ public Headers put(String key, Collection values) { - if (isReadOnly()) { + if (readOnly) { throw new UnsupportedOperationException(); } if (values == null || values.isEmpty()) { @@ -191,7 +193,7 @@ public Headers put(String key, Collection values) { * @return the Headers object */ public Headers put(Map> map) { - if (isReadOnly()) { + if (readOnly) { throw new UnsupportedOperationException(); } if (map == null || map.isEmpty()) { @@ -228,7 +230,7 @@ private Headers _put(String key, Collection values) { * @param keys the key or keys to remove */ public void remove(String... keys) { - if (isReadOnly()) { + if (readOnly) { throw new UnsupportedOperationException(); } for (String key : keys) { @@ -243,7 +245,7 @@ public void remove(String... keys) { * @param keys the key or keys to remove */ public void remove(Collection keys) { - if (isReadOnly()) { + if (readOnly) { throw new UnsupportedOperationException(); } for (String key : keys) { @@ -282,7 +284,7 @@ public boolean isEmpty() { * Removes all the keys The object map will be empty after this call returns. */ public void clear() { - if (isReadOnly()) { + if (readOnly) { throw new UnsupportedOperationException(); } valuesMap.clear(); @@ -577,7 +579,7 @@ boolean hasValues() { * @return the read only state */ public boolean isReadOnly() { - return !(valuesMap instanceof HashMap); + return readOnly; } @Override From cd4f4d25355d091dd6b27a42226acc48a454a69c Mon Sep 17 00:00:00 2001 From: "A.Fink" Date: Tue, 12 Aug 2025 03:19:15 +0300 Subject: [PATCH 03/10] refactoring after review --- .../java/io/nats/client/impl/Headers.java | 42 ++++++++++--------- .../io/nats/client/impl/HeadersTests.java | 6 ++- 2 files changed, 27 insertions(+), 21 deletions(-) diff --git a/src/main/java/io/nats/client/impl/Headers.java b/src/main/java/io/nats/client/impl/Headers.java index 666af2fb8..b71b1ce0f 100644 --- a/src/main/java/io/nats/client/impl/Headers.java +++ b/src/main/java/io/nats/client/impl/Headers.java @@ -1,4 +1,4 @@ -// Copyright 2020 The NATS Authors +// Copyright 2020-2025 The NATS Authors // 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: @@ -18,10 +18,7 @@ import java.util.*; import java.util.function.BiConsumer; -import java.util.stream.Collectors; - import static io.nats.client.support.NatsConstants.*; -import static java.nio.charset.StandardCharsets.US_ASCII; /** * An object that represents a map of keys to a list of values. It does not accept @@ -403,8 +400,9 @@ public Set keySetIgnoreCase() { * removed during iteration */ public void forEach(BiConsumer> action) { - valuesMap.forEach((key, values) -> - action.accept(key, Collections.unmodifiableList(values))); + for (Map.Entry> entry : valuesMap.entrySet()) { + action.accept(entry.getKey(), Collections.unmodifiableList(entry.getValue())); + } } /** @@ -506,6 +504,7 @@ public int serializeToArray(int destPosition, byte[] dest) { dest[destPosition++] = CR; dest[destPosition] = LF; + //to do update serialized and/or dataLength? return serializedLength(); } @@ -525,7 +524,8 @@ private void checkKey(String key) { for (int idx = 0; idx < len; idx++) { char c = key.charAt(idx); if (c < 33 || c > 126 || c == ':') { - throw new IllegalArgumentException(KEY_INVALID_CHARACTER + "'" + c + "'"); + throw new IllegalArgumentException(KEY_INVALID_CHARACTER +"0x"+ Integer.toHexString(c) + +" at "+ idx +" in "+ key); } } } @@ -541,7 +541,8 @@ private void checkValue(String val) { for (int i = 0, len = val.length(); i < len; i++) { int c = val.charAt(i); if (c > 127 || c == 10 || c == 13) { - throw new IllegalArgumentException(VALUE_INVALID_CHARACTERS + Integer.toHexString(c)); + throw new IllegalArgumentException(VALUE_INVALID_CHARACTERS +"0x"+ Integer.toHexString(c) + +" at "+ i +" in "+ val); } } } @@ -595,18 +596,21 @@ public int hashCode() { return Objects.hashCode(valuesMap); } + @SuppressWarnings("deprecation") @Override public String toString() { - return valuesMap.entrySet().stream() - .filter(e -> e.getValue() != null) - .sorted(Map.Entry.comparingByKey()) - .map(e -> { - String headerName = e.getKey(); - List values = e.getValue(); - return headerName +": "+ (values.size() == 1 ? values.get(0) - : String.join(", ", values) - ); - }) - .collect(Collectors.joining("; ")); + byte[] b = getSerialized(); + if (b.length <= HVCRLF_BYTES + 2){ + return ""; + } + + for (int i = 0, len = b.length; i < len; i++) { + if (b[i] == LF) { + b[i] = ' '; + } else if (b[i] == CR) { + b[i] = ';'; + } + } + return new String(b, 0, HVCRLF_BYTES, b.length - HVCRLF_BYTES - 3); } } diff --git a/src/test/java/io/nats/client/impl/HeadersTests.java b/src/test/java/io/nats/client/impl/HeadersTests.java index 8e6da6924..be0bc186b 100644 --- a/src/test/java/io/nats/client/impl/HeadersTests.java +++ b/src/test/java/io/nats/client/impl/HeadersTests.java @@ -769,14 +769,16 @@ public void testToString() { assertNotNull(new Status(1, "msg").toString()); // COVERAGE Headers h = new Headers(); + assertEquals("", h.toString()); + h.add("Test1"); h.add("Test2", "Test2Value"); h.add("Test3", ""); h.add("Test4", "", "", ""); h.add("Test5", "Nice!", "To.", "See?"); - assertEquals("Test2: Test2Value; Test3: ; Test4: , , ; Test5: Nice!, To., See?", h.toString() - ); + assertEquals("Test5:Nice!; Test5:To.; Test5:See?; Test4:; Test4:; Test4:; Test3:; Test2:Test2Value;", + h.toString());// flaky: non-sorted HashMap } @Test From 2475934901dc08f96274c2a45322f6879a2b011c Mon Sep 17 00:00:00 2001 From: "A.Fink" Date: Tue, 12 Aug 2025 03:22:19 +0300 Subject: [PATCH 04/10] much faster serializeToArray 1) getBytes 3-5x faster than getBytes(US_ASCII) 2) don't create and copy extra byte arrays --- src/main/java/io/nats/client/impl/Headers.java | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/src/main/java/io/nats/client/impl/Headers.java b/src/main/java/io/nats/client/impl/Headers.java index b71b1ce0f..ded28dd02 100644 --- a/src/main/java/io/nats/client/impl/Headers.java +++ b/src/main/java/io/nats/client/impl/Headers.java @@ -480,22 +480,21 @@ public ByteArrayBuilder appendSerialized(ByteArrayBuilder bab) { * @param dest the byte array to write to * @return the length of the header */ + @SuppressWarnings("deprecation") public int serializeToArray(int destPosition, byte[] dest) { System.arraycopy(HEADER_VERSION_BYTES_PLUS_CRLF, 0, dest, destPosition, HVCRLF_BYTES); destPosition += HVCRLF_BYTES; for (Map.Entry> entry : valuesMap.entrySet()) { - List values = entry.getValue(); - for (String value : values) { - byte[] bytes = entry.getKey().getBytes(US_ASCII); - System.arraycopy(bytes, 0, dest, destPosition, bytes.length); - destPosition += bytes.length; + String key = entry.getKey(); + for (String value : entry.getValue()) { + key.getBytes(0, key.length(), dest, destPosition);// key has only US_ASCII + destPosition += key.length(); dest[destPosition++] = COLON; - bytes = value.getBytes(US_ASCII); - System.arraycopy(bytes, 0, dest, destPosition, bytes.length); - destPosition += bytes.length; + value.getBytes(0, value.length(), dest, destPosition); + destPosition += value.length(); dest[destPosition++] = CR; dest[destPosition++] = LF; From 19122d09874e74cc6e33b99c9869f6c83b075a11 Mon Sep 17 00:00:00 2001 From: "A.Fink" Date: Tue, 12 Aug 2025 03:43:51 +0300 Subject: [PATCH 05/10] benchmark: new serializeToArray is 3.7 times faster --- .../io/nats/client/impl/HeadersTests.java | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/src/test/java/io/nats/client/impl/HeadersTests.java b/src/test/java/io/nats/client/impl/HeadersTests.java index be0bc186b..a8b018acd 100644 --- a/src/test/java/io/nats/client/impl/HeadersTests.java +++ b/src/test/java/io/nats/client/impl/HeadersTests.java @@ -826,4 +826,32 @@ void testCheckValue() { assertEquals(1, h.get("test1").size()); assertEquals("\u0000 \f\b\t", h.getFirst("test1")); } + + /** + no JMH :( + Old: Time: 24622.87ms, Op/sec: 4061264 + New: Time: 6660.177ms, Op/sec: 15014614 + New variant is 15014614/4061264= 3.7 times faster + */ + @Test + void benchmark_serializeToArray() { + Headers h = new Headers().put("test", "aaa", "bBb", "ZZZZZZZZ") + .put("ALongLongLongLongLongLongLongKey", "VeryLongLongLongLongLongLongLongLongLong:Value!"); + assertEquals( + "ALongLongLongLongLongLongLongKey:VeryLongLongLongLongLongLongLongLongLong:Value!; test:aaa; test:bBb; test:ZZZZZZZZ;", + h.toString()); + + byte[] dst = new byte[1000]; + for (int i = 0; i < 10_000; i++) {// warm-up + assertEquals(129, h.serializeToArray(0, dst)); + } + + long t = System.nanoTime(); + int max = 100_000_000; + for (int i = 0; i < max; i++) { + h.serializeToArray(0, dst); + } + t = System.nanoTime() - t; + System.out.println("Time: " + t / 1000 / 1000.0 +"ms, Op/sec: "+(max*1_000_000_000L/t)); + } } From 69168712ae28e9652724b2882226fc4bd1507b57 Mon Sep 17 00:00:00 2001 From: "A.Fink" Date: Tue, 12 Aug 2025 12:16:33 +0300 Subject: [PATCH 06/10] disable benchmark --- src/test/java/io/nats/client/impl/HeadersTests.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/test/java/io/nats/client/impl/HeadersTests.java b/src/test/java/io/nats/client/impl/HeadersTests.java index a8b018acd..f6b2c05ac 100644 --- a/src/test/java/io/nats/client/impl/HeadersTests.java +++ b/src/test/java/io/nats/client/impl/HeadersTests.java @@ -4,6 +4,7 @@ import io.nats.client.support.Status; import io.nats.client.support.Token; import io.nats.client.support.TokenType; +import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import java.nio.charset.StandardCharsets; @@ -830,10 +831,10 @@ void testCheckValue() { /** no JMH :( Old: Time: 24622.87ms, Op/sec: 4061264 - New: Time: 6660.177ms, Op/sec: 15014614 + New: Time: 6660.18ms, Op/sec: 15014614 New variant is 15014614/4061264= 3.7 times faster */ - @Test + @Test @Disabled("Benchmark after changes in serializeToArray: Time: 6_660ms, Op/sec: 15_014_614") void benchmark_serializeToArray() { Headers h = new Headers().put("test", "aaa", "bBb", "ZZZZZZZZ") .put("ALongLongLongLongLongLongLongKey", "VeryLongLongLongLongLongLongLongLongLong:Value!"); From 5893a7a95e23b278ea5e941bf1ff99fdcb8f05af Mon Sep 17 00:00:00 2001 From: "A.Fink" Date: Tue, 12 Aug 2025 15:29:19 +0300 Subject: [PATCH 07/10] simplify validation message after review "Just this please. private static final String KEY_INVALID_CHARACTER = "Header key has invalid character: 0x"; throw new IllegalArgumentException(KEY_INVALID_CHARACTER + Integer.toHexString(c));" --- src/main/java/io/nats/client/impl/Headers.java | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/main/java/io/nats/client/impl/Headers.java b/src/main/java/io/nats/client/impl/Headers.java index ded28dd02..a54669d45 100644 --- a/src/main/java/io/nats/client/impl/Headers.java +++ b/src/main/java/io/nats/client/impl/Headers.java @@ -30,8 +30,8 @@ public class Headers { private static final String KEY_CANNOT_BE_EMPTY_OR_NULL = "Header key cannot be null."; - private static final String KEY_INVALID_CHARACTER = "Header key has invalid character: "; - private static final String VALUE_INVALID_CHARACTERS = "Header value has invalid character: "; + private static final String KEY_INVALID_CHARACTER = "Header key has invalid character: 0x"; + private static final String VALUE_INVALID_CHARACTERS = "Header value has invalid character: 0x"; private final Map> valuesMap; private final Map lengthMap; @@ -523,8 +523,7 @@ private void checkKey(String key) { for (int idx = 0; idx < len; idx++) { char c = key.charAt(idx); if (c < 33 || c > 126 || c == ':') { - throw new IllegalArgumentException(KEY_INVALID_CHARACTER +"0x"+ Integer.toHexString(c) - +" at "+ idx +" in "+ key); + throw new IllegalArgumentException(KEY_INVALID_CHARACTER + Integer.toHexString(c)); } } } @@ -540,8 +539,7 @@ private void checkValue(String val) { for (int i = 0, len = val.length(); i < len; i++) { int c = val.charAt(i); if (c > 127 || c == 10 || c == 13) { - throw new IllegalArgumentException(VALUE_INVALID_CHARACTERS +"0x"+ Integer.toHexString(c) - +" at "+ i +" in "+ val); + throw new IllegalArgumentException(VALUE_INVALID_CHARACTERS + Integer.toHexString(c)); } } } From 3788bb15a6ad33884ba78f49626003ca2b2b4a4e Mon Sep 17 00:00:00 2001 From: "A.Fink" Date: Tue, 12 Aug 2025 15:35:33 +0300 Subject: [PATCH 08/10] simplify toString after review --- .../java/io/nats/client/impl/Headers.java | 21 +++++++++---------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/src/main/java/io/nats/client/impl/Headers.java b/src/main/java/io/nats/client/impl/Headers.java index a54669d45..06fb04b14 100644 --- a/src/main/java/io/nats/client/impl/Headers.java +++ b/src/main/java/io/nats/client/impl/Headers.java @@ -16,6 +16,7 @@ import io.nats.client.support.ByteArrayBuilder; import org.jspecify.annotations.Nullable; +import java.nio.charset.StandardCharsets; import java.util.*; import java.util.function.BiConsumer; import static io.nats.client.support.NatsConstants.*; @@ -593,21 +594,19 @@ public int hashCode() { return Objects.hashCode(valuesMap); } - @SuppressWarnings("deprecation") @Override public String toString() { byte[] b = getSerialized(); - if (b.length <= HVCRLF_BYTES + 2){ - return ""; - } - - for (int i = 0, len = b.length; i < len; i++) { - if (b[i] == LF) { - b[i] = ' '; - } else if (b[i] == CR) { - b[i] = ';'; + int len = b.length; + if (len <= HVCRLF_BYTES + 2){ + return "";// empty map + } + for (int i = 0; i < len; i++) { + switch (b[i]) { + case CR: b[i] = ';'; break; + case LF: b[i] = ' '; break; } } - return new String(b, 0, HVCRLF_BYTES, b.length - HVCRLF_BYTES - 3); + return new String(b, HVCRLF_BYTES, len - HVCRLF_BYTES - 3, StandardCharsets.ISO_8859_1);// b has only US_ASCII, ISO_8859_1 is 3x faster } } From 5e682c76b7b043fd41eefdf32efcb52c930ced55 Mon Sep 17 00:00:00 2001 From: "A.Fink" Date: Tue, 12 Aug 2025 23:06:40 +0300 Subject: [PATCH 09/10] optimize Headers.Checker after review https://github.com/nats-io/nats.java/pull/1385#issuecomment-3180190678 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 1) static class Checker doesn't contain hidden reference to Headers ⇒ -1 field 👍 2) not private checkKey and checkValue can be accessed directly from Checker, without synthetic accessors 3) static should be a little faster --- src/main/java/io/nats/client/impl/Headers.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/main/java/io/nats/client/impl/Headers.java b/src/main/java/io/nats/client/impl/Headers.java index 06fb04b14..c094d1b10 100644 --- a/src/main/java/io/nats/client/impl/Headers.java +++ b/src/main/java/io/nats/client/impl/Headers.java @@ -514,7 +514,7 @@ public int serializeToArray(int destPosition, byte[] dest) { * @throws IllegalArgumentException if the key is null, empty or contains * an invalid character */ - private void checkKey(String key) { + static void checkKey(String key) { // key cannot be null or empty and contain only printable characters except colon if (key == null || key.isEmpty()) { throw new IllegalArgumentException(KEY_CANNOT_BE_EMPTY_OR_NULL); @@ -534,7 +534,7 @@ private void checkKey(String key) { * * @throws IllegalArgumentException if the value contains an invalid character */ - private void checkValue(String val) { + static void checkValue(String val) { // Like rfc822 section 3.1.2 (quoted in ADR 4) // The field-body may be composed of any US-ASCII characters, except CR or LF. for (int i = 0, len = val.length(); i < len; i++) { @@ -545,7 +545,7 @@ private void checkValue(String val) { } } - private class Checker { + private static final class Checker { List list = new ArrayList<>(); int len = 0; From 29e3088640dc78ff153770e8d69f22a1776afe10 Mon Sep 17 00:00:00 2001 From: "A.Fink" Date: Wed, 13 Aug 2025 00:11:29 +0300 Subject: [PATCH 10/10] fix after review --- src/main/java/io/nats/client/impl/Headers.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/main/java/io/nats/client/impl/Headers.java b/src/main/java/io/nats/client/impl/Headers.java index c094d1b10..b506bd7e9 100644 --- a/src/main/java/io/nats/client/impl/Headers.java +++ b/src/main/java/io/nats/client/impl/Headers.java @@ -331,7 +331,7 @@ public Set keySet() { * @return a read-only set of keys (in lowercase) contained in this map */ public Set keySetIgnoreCase() { - HashSet set = new HashSet<>(valuesMap.size()*4/3 + 1); + HashSet set = new HashSet<>();// no capacity is OK for small maps for (String k : valuesMap.keySet()) { set.add(k.toLowerCase()); } @@ -476,7 +476,9 @@ public ByteArrayBuilder appendSerialized(ByteArrayBuilder bab) { /** * Write the header to the byte array. Assumes that the caller has - * already validated that the destination array is large enough by using getSerialized() + * already validated that the destination array is large enough by using {@link #getSerialized()}. + *

/Deprecated {@link String#getBytes(int, int, byte[], int)} is used, because it still exists in JDK 25 + * and is 10–30 times faster than {@code getBytes(ISO_8859_1/US_ASCII)}/ * @param destPosition the position index in destination byte array to start * @param dest the byte array to write to * @return the length of the header