From 49870b9716813dbde37c645aea9c118cf9439eeb Mon Sep 17 00:00:00 2001 From: Alan Zimmer <48699787+alzimmermsft@users.noreply.github.com> Date: Thu, 19 Sep 2024 17:00:15 -0400 Subject: [PATCH] Reduce synchronization lock contention in common code paths (#41816) Reduce synchronization lock contention in common code paths --- .../java/com/azure/core/util/UrlBuilder.java | 32 ++++++++++++++++++- .../common/StorageSharedKeyCredential.java | 22 ++++++++----- 2 files changed, 45 insertions(+), 9 deletions(-) diff --git a/sdk/core/azure-core/src/main/java/com/azure/core/util/UrlBuilder.java b/sdk/core/azure-core/src/main/java/com/azure/core/util/UrlBuilder.java index 36a8731b2da19..6ccc831f181f4 100644 --- a/sdk/core/azure-core/src/main/java/com/azure/core/util/UrlBuilder.java +++ b/sdk/core/azure-core/src/main/java/com/azure/core/util/UrlBuilder.java @@ -18,8 +18,11 @@ /** * A builder class that is used to create URLs. */ +@SuppressWarnings("deprecation") public final class UrlBuilder { private static final Map PARSED_URLS = new ConcurrentHashMap<>(); + private static final URL HTTP; + private static final URL HTTPS; private String scheme; private String host; @@ -29,6 +32,18 @@ public final class UrlBuilder { private Map queryToCopy; private Map query; + static { + try { + // Instantiate two URLs, one with HTTP scheme and one with HTTPS scheme. + // These will be used when creating HTTP and HTTPS URLs respectively when calling 'toUrl' to improve + // performance in highly threaded situations. + HTTP = new URL("http://azure.com"); + HTTPS = new URL("https://azure.com"); + } catch (MalformedURLException e) { + throw new RuntimeException(e); + } + } + /** * Creates a new instance of {@link UrlBuilder}. */ @@ -330,7 +345,22 @@ private UrlBuilder with(String text, UrlTokenizerState startState) { public URL toUrl() throws MalformedURLException { // Continue using new URL constructor here as URI either cannot accept certain characters in the path or // escapes '/', depending on the API used to create the URI. - return ImplUtils.createUrl(toString()); + if ("http".equals(scheme)) { + // Performance enhancement. Using "new URL(URL context, String spec)" circumvents a lookup for the + // URLStreamHandler when creating the URL instance. In highly threaded environments this can cause slowdown + // as this lookup uses a HashTable which is synchronized, resulting in many threads waiting on the same + // lock. + // Select the URL constant that matches the scheme as it is possible that the URLStreamHandler may be needed + // in downstream scenarios. + // Using this performance enhancement should be safe as the "String spec" will override any URL pieces from + // "URL context" when it is parsed, and the "context" URL we're using here only has scheme and hostname + // which should always be configured in UrlBuilder. + return new URL(HTTP, toString()); + } else if ("https".equals(scheme)) { + return new URL(HTTPS, toString()); + } else { + return ImplUtils.createUrl(toString()); + } } /** diff --git a/sdk/storage/azure-storage-common/src/main/java/com/azure/storage/common/StorageSharedKeyCredential.java b/sdk/storage/azure-storage-common/src/main/java/com/azure/storage/common/StorageSharedKeyCredential.java index be59f008dec28..fe971a163dbd0 100644 --- a/sdk/storage/azure-storage-common/src/main/java/com/azure/storage/common/StorageSharedKeyCredential.java +++ b/sdk/storage/azure-storage-common/src/main/java/com/azure/storage/common/StorageSharedKeyCredential.java @@ -34,6 +34,12 @@ public final class StorageSharedKeyCredential { private static final ClientLogger LOGGER = new ClientLogger(StorageSharedKeyCredential.class); + // Previous design used a constant for the ROOT_COLLATOR. This runs into performance issues as the ROOT Collator + // can have the comparison method synchronized. In highly threaded environments this can result in threads waiting + // to enter the synchronized block. + private static final ThreadLocal THREAD_LOCAL_COLLATOR + = ThreadLocal.withInitial(() -> Collator.getInstance(Locale.ROOT)); + private static final Context LOG_STRING_TO_SIGN_CONTEXT = new Context(Constants.STORAGE_LOG_STRING_TO_SIGN, true); // Pieces of the connection string that are needed. @@ -41,7 +47,6 @@ public final class StorageSharedKeyCredential { private static final String ACCOUNT_NAME = "accountname"; private static final HttpHeaderName X_MS_DATE = HttpHeaderName.fromString("x-ms-date"); - private static final Collator ROOT_COLLATOR = Collator.getInstance(Locale.ROOT); private final AzureNamedKeyCredential azureNamedKeyCredential; @@ -185,6 +190,7 @@ private String buildStringToSign(URL requestURL, String httpMethod, HttpHeaders String dateHeader = (headers.getValue(X_MS_DATE) != null) ? "" : getStandardHeaderValue(headers, HttpHeaderName.DATE); + Collator collator = THREAD_LOCAL_COLLATOR.get(); String stringToSign = String.join("\n", httpMethod, getStandardHeaderValue(headers, HttpHeaderName.CONTENT_ENCODING), @@ -198,8 +204,8 @@ private String buildStringToSign(URL requestURL, String httpMethod, HttpHeaders getStandardHeaderValue(headers, HttpHeaderName.IF_NONE_MATCH), getStandardHeaderValue(headers, HttpHeaderName.IF_UNMODIFIED_SINCE), getStandardHeaderValue(headers, HttpHeaderName.RANGE), - getAdditionalXmsHeaders(headers), - getCanonicalizedResource(requestURL)); + getAdditionalXmsHeaders(headers, collator), + getCanonicalizedResource(requestURL, collator)); if (logStringToSign) { StorageImplUtils.logStringToSign(LOGGER, stringToSign, LOG_STRING_TO_SIGN_CONTEXT); @@ -216,7 +222,7 @@ private String getStandardHeaderValue(HttpHeaders headers, HttpHeaderName header return header == null ? "" : header.getValue(); } - private static String getAdditionalXmsHeaders(HttpHeaders headers) { + private static String getAdditionalXmsHeaders(HttpHeaders headers, Collator collator) { List
xmsHeaders = new ArrayList<>(); int stringBuilderSize = 0; @@ -239,7 +245,7 @@ private static String getAdditionalXmsHeaders(HttpHeaders headers) { final StringBuilder canonicalizedHeaders = new StringBuilder( stringBuilderSize + (2 * xmsHeaders.size()) - 1); - xmsHeaders.sort((o1, o2) -> ROOT_COLLATOR.compare(o1.getName(), o2.getName())); + xmsHeaders.sort((o1, o2) -> collator.compare(o1.getName(), o2.getName())); for (Header xmsHeader : xmsHeaders) { if (canonicalizedHeaders.length() > 0) { @@ -253,7 +259,7 @@ private static String getAdditionalXmsHeaders(HttpHeaders headers) { return canonicalizedHeaders.toString(); } - private String getCanonicalizedResource(URL requestURL) { + private String getCanonicalizedResource(URL requestURL, Collator collator) { // Resource path String resourcePath = azureNamedKeyCredential.getAzureNamedKey().getName(); @@ -277,7 +283,7 @@ private String getCanonicalizedResource(URL requestURL) { // // Example 1: prefix=a%2cb => prefix={decode(a%2cb)} => prefix={"a,b"} // Example 2: prefix=a,2 => prefix={decode(a),decode(b) => prefix={"a","b"} - TreeMap> pieces = new TreeMap<>(ROOT_COLLATOR); + TreeMap> pieces = new TreeMap<>(collator); StorageImplUtils.parseQueryParameters(query).forEachRemaining(kvp -> { String key = urlDecode(kvp.getKey()).toLowerCase(Locale.ROOT); @@ -302,7 +308,7 @@ private String getCanonicalizedResource(URL requestURL) { for (Map.Entry> queryParam : pieces.entrySet()) { List queryParamValues = queryParam.getValue(); - queryParamValues.sort(ROOT_COLLATOR); + queryParamValues.sort(collator); canonicalizedResource.append('\n').append(queryParam.getKey()).append(':'); int size = queryParamValues.size();