Performance: Direct code path performance improvements (both memory and CPU) #14147

kirankumarkolli · 2020-08-14T22:51:24Z

O(n) headers iteration in StoreResult for ReqeustCharge (DirectConnectivity header names are always populated inside SDK)
O(n) Map headers to arrays and then reverse conversion overhead (including memory) StoreResponse
Extra map materialization in StoreClient
Avoid intermediate List materializations in RntbdResponseHeaders
O(n) -O(1) handling for OWNER_FULL_NAME_VALUE header value

DO_NOT_MERGE

- O(n) headers iteration in StoreResult for ReqeustCharge (DirectConnectivity header names are always populated inside SDK) - O(n) Map headers to arrays and then reverse conversion overhead (including memory) StoreResponse - Extra map materialization in StoreClient - Avoid intermediate List<Entry> materializations in RntbdResponseHeaders - O(n) -O(1) handling for OWNER_FULL_NAME_VALUE header value

kirankumarkolli · 2020-08-14T22:52:48Z

Yet to run the performance benchmark

- O(n) headers iteration in StoreResult for ReqeustCharge (DirectConnectivity header names are always populated inside SDK) - O(n) Map headers to arrays and then reverse conversion overhead (including memory) StoreResponse - Extra map materialization in StoreClient - Avoid intermediate List<Entry> materializations in RntbdResponseHeaders - O(n) -O(1) handling for OWNER_FULL_NAME_VALUE header value

….com/Azure/azure-sdk-for-java into users/kirankk/store_response_headers

moderakh · 2020-08-17T16:30:06Z

...azure-cosmos/src/main/java/com/azure/cosmos/implementation/directconnectivity/HttpUtils.java

-            result.add(entry);
+    public static void unescape(Map<String, String> headers) {
+        if (headers != null) {
+            headers.computeIfPresent(HttpConstants.HttpHeaders.OWNER_FULL_NAME,


very good change. thanks.

moderakh · 2020-08-17T16:40:33Z

...smos/src/test/java/com/azure/cosmos/implementation/directconnectivity/StoreResponseTest.java

@@ -19,7 +19,7 @@ public void stringContent() {
        headerMap.put("key1", "value1");
        headerMap.put("key2", "value2");

-        StoreResponse sp = new StoreResponse(200, new ArrayList<>(headerMap.entrySet()), getUTF8BytesOrNull(content));
+        StoreResponse sp = new StoreResponse(200,headerMap, getUTF8BytesOrNull(content));


code style space after comma

Suggested change

StoreResponse sp = new StoreResponse(200,headerMap, getUTF8BytesOrNull(content));

StoreResponse sp = new StoreResponse(200, headerMap, getUTF8BytesOrNull(content));

moderakh · 2020-08-17T16:42:18Z

.../src/test/java/com/azure/cosmos/implementation/directconnectivity/ConsistencyWriterTest.java

-import java.util.Collections;
-import java.util.List;
-import java.util.Map;
+import java.util.*;


code style, no wildcard import:
https://peterdev.pl/2019/10/18/why-you-should-avoid-star-imports-in-intellij-idea/

moderakh · 2020-08-17T16:45:45Z

...ure-cosmos/src/main/java/com/azure/cosmos/implementation/directconnectivity/StoreResult.java

-                }
-            }
+        else if (response.getResponseHeaders() != null) {
+            response.getResponseHeaders().put(HttpConstants.HttpHeaders.REQUEST_CHARGE, Double.toString(totalRequestCharge));


Rntbd generated response headerMap is ImmutableMap.
now that the intermediate copied version is removed are we modifying the rntbd header map directly? if so this may not work.

Could you please check.

moderakh · 2020-08-17T16:47:59Z

...main/java/com/azure/cosmos/implementation/directconnectivity/rntbd/RntbdResponseHeaders.java

-            toBooleanEntry(BackendHeaders.OFFER_REPLACE_PENDING, token)
-        );
+    private Map<String, String> collectEntries(final RntbdContext context, final UUID activityId) {
+        Map<String, String> responseHeaders = new HashMap<>(this.computeCount() + 2);


I remember I tested this a few months back and I found out that guava ImmutableMap construction is more performant than HashMap construction.
in my old perf testing replacing ImmutableMap with HashMap resulted in perf drop.

Could you please do a fully e2e perf testing to compare the perf difference of this PR?

AKS: Review request for Microsoft.ContainerService to add version stable/2021-05-01 (Azure#14147) * Adds base for updating Microsoft.ContainerService from version stable/2021-03-01 to version 2021-05-01 * Updates readme * Updates API version in new specs and examples * add APIServerAccessProfile.enablePrivateClusterPublicFQDN in 2021-05-01 (Azure#14149) * add APIServerAccessProfile.enablePrivateClusterPublicFQDN in 2021-05-01 api * ref example Co-authored-by: Li Ma <[email protected]> * Add swagger for OutboundNetworkDependenciesEndpoints for AKS (Azure#14332) * Add swagger for OutboundNetworkDependenciesEndpoints for AKS * fix * lint * update operationId * aks: add enableUltraSSD in v20210501 api-version (Azure#14354) * update doc for agentpool max count from 100 to 1000 (Azure#14553) Co-authored-by: Li Ma <[email protected]> * add readme for new version (Azure#14611) Co-authored-by: Li Ma <[email protected]> * fix conflict (Azure#14614) Co-authored-by: Li Ma <[email protected]> Co-authored-by: Li Ma <[email protected]> Co-authored-by: gossion <[email protected]> Co-authored-by: Andy Zhang <[email protected]>

ghost · 2022-02-25T10:06:50Z

Hi @kirankumarkolli. Thank you for your interest in helping to improve the Azure SDK experience and for your contribution. We've noticed that there hasn't been recent engagement on this pull request. If this is still an active work stream, please let us know by pushing some changes or leaving a comment. Otherwise, we'll close this out in 7 days.

ghost · 2022-03-04T11:05:00Z

Hi @kirankumarkolli. Thank you for your contribution. Since there hasn't been recent engagement, we're going to close this out. Feel free to respond with a comment containing "/reopen" if you'd like to continue working on these changes. Please be sure to use the command to reopen or remove the "no-recent-activity" label; otherwise, this is likely to be closed again with the next cleanup pass.

kirankumarkolli requested review from David-Noble-at-work, kushagraThapar, mbhaskar, milismsft, moderakh, simplynaveen20 and xinlian12 as code owners August 14, 2020 22:51

ghost added the Cosmos label Aug 14, 2020

kirankumarkolli added 3 commits August 15, 2020 12:28

Merge branch 'users/kirankk/store_response_headers' of https://github…

178f33e

….com/Azure/azure-sdk-for-java into users/kirankk/store_response_headers

Removing dead code

3f3f67d

kirankumarkolli requested review from anuchandy, chenrujun, danieljurek, g2vinay, hemanttanwar, jialindai, JimSuplizio, JonathanGiles, joshfree, mitchdenny, mssfang, samvaity, saragluna, srnagar, vcolin7, weshaggard and yiliuTo as code owners August 17, 2020 04:28

kirankumarkolli requested a review from zhoufenqin as a code owner August 17, 2020 04:28

Merge branch 'master' into users/kirankk/store_response_headers

9850784

kirankumarkolli removed request for mitchdenny, joshfree, JonathanGiles, danieljurek, g2vinay, vcolin7, hemanttanwar, zhoufenqin, weshaggard, chenrujun, JimSuplizio, samvaity, saragluna, mssfang, srnagar, jialindai and yiliuTo August 17, 2020 05:45

moderakh reviewed Aug 17, 2020

View reviewed changes

JonathanGiles assigned kirankumarkolli Aug 6, 2021

ghost added the no-recent-activity There has been no recent activity on this issue. label Feb 25, 2022

ghost closed this Mar 4, 2022

conniey deleted the users/kirankk/store_response_headers branch March 17, 2022 19:15

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Performance: Direct code path performance improvements (both memory and CPU) #14147

Performance: Direct code path performance improvements (both memory and CPU) #14147

kirankumarkolli commented Aug 14, 2020 •

edited

Loading

kirankumarkolli commented Aug 14, 2020

moderakh Aug 17, 2020

moderakh Aug 17, 2020

moderakh Aug 17, 2020

moderakh Aug 17, 2020

moderakh Aug 17, 2020

ghost commented Feb 25, 2022

ghost commented Mar 4, 2022

	StoreResponse sp = new StoreResponse(200,headerMap, getUTF8BytesOrNull(content));
	StoreResponse sp = new StoreResponse(200, headerMap, getUTF8BytesOrNull(content));

Performance: Direct code path performance improvements (both memory and CPU) #14147

Performance: Direct code path performance improvements (both memory and CPU) #14147

Conversation

kirankumarkolli commented Aug 14, 2020 • edited Loading

kirankumarkolli commented Aug 14, 2020

moderakh Aug 17, 2020

Choose a reason for hiding this comment

moderakh Aug 17, 2020

Choose a reason for hiding this comment

moderakh Aug 17, 2020

Choose a reason for hiding this comment

moderakh Aug 17, 2020

Choose a reason for hiding this comment

moderakh Aug 17, 2020

Choose a reason for hiding this comment

ghost commented Feb 25, 2022

ghost commented Mar 4, 2022

kirankumarkolli commented Aug 14, 2020 •

edited

Loading