Reduce synchronization lock contention in common code paths (#41816)

Reduce synchronization lock contention in common code paths

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<String, UrlBuilder> 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<String, QueryParameter> queryToCopy;
     private Map<String, QueryParameter> 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());
+        }
     }
 
     /**

sdk/storage/azure-storage-common/src/main/java/com/azure/storage/common/StorageSharedKeyCredential.java

@@ -34,14 +34,19 @@
 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<Collator> 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.
     private static final String ACCOUNT_KEY = "accountkey";
     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<Header> 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<String, List<String>> pieces = new TreeMap<>(ROOT_COLLATOR);
+        TreeMap<String, List<String>> 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<String, List<String>> queryParam : pieces.entrySet()) {
             List<String> queryParamValues = queryParam.getValue();
-            queryParamValues.sort(ROOT_COLLATOR);
+            queryParamValues.sort(collator);
             canonicalizedResource.append('\n').append(queryParam.getKey()).append(':');
 
             int size = queryParamValues.size();