Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

extending String de-optimizes method dispatch / Importing AWS SDK DynamoDB affects performance of third party library #6182

Closed
3 tasks done
sazarkin opened this issue Jun 10, 2024 · 11 comments
Assignees
Labels
bug This issue is a bug. p1 This is a high priority issue queued This issues is on the AWS team's backlog

Comments

@sazarkin
Copy link

sazarkin commented Jun 10, 2024

Checkboxes for prior research

Describe the bug

After migrating from v2 to v3 I've noticed slowdown of one of our metrics.
Importing dynamodb client affects parsing of html files.

What exactly aws sdk v3 does on import? Is it changes some runtime params? How to turn it off?

SDK version number

@aws-sdk/[email protected]

Which JavaScript Runtime is this issue in?

Node.js

Details of the browser/Node.js/ReactNative version

v20.13.1

Reproduction Steps

"use strict";
const initStart = performance.now();
const parse5_1 = require("parse5");

if (process.env.AWS_SDK === '1') {
    const client_dynamodb_1 = require("@aws-sdk/client-dynamodb");
    const version = require('@aws-sdk/client-dynamodb/package.json').version;
    console.log('DynamoDB', typeof client_dynamodb_1.DynamoDB, version);
}

const html = `<doctype html><html><head><title>Test</title></head><body><h1>Hello World</h1></body></html>`;

async function benchParse(count) {
    for (let i = 0; i < count; i++) {
        const doc = (0, parse5_1.parse)(html);
        (0, parse5_1.serialize)(doc);
    }
}

const name = 'bench-parse5';
const count = 1000000;
const benchFunc = benchParse;
const initDone = performance.now();
console.log('---', name, `init ${(initDone - initStart).toFixed(2)}ms`);
const startTime = performance.now();
console.profile(name);
benchFunc(count).then(() => {
    console.profileEnd();
    const grandTotal = performance.now() - startTime;
    console.log(`Count: ${count} Time: ${grandTotal.toFixed(2)}ms`);
    console.log('---');
});
{
  "dependencies": {
    "@aws-sdk/client-dynamodb": "^3.592.0",
    "parse5": "^7.1.2"
  }
}

Observed Behavior

with dynamodb import

DynamoDB function 3.592.0
--- bench-parse5 init 66.59ms
Count: 1000000 Time: 4507.70ms
---

Expected Behavior

test output: around 800ms lower

--- bench-parse5 init 12.97ms
Count: 1000000 Time: 3730.11ms
---

Possible Solution

No response

Additional Information/Context

No response

@sazarkin sazarkin added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Jun 10, 2024
@aBurmeseDev
Copy link
Member

Hi @sazarkin - thanks for reaching out.

I'd like to understand what your workflow looks like, and how you're using SDK. In your code example, I see that you're client-dynamodb but it doesn't show where the client call is being made. Please share steps to repro and complete SDK code example that will give us more insight.

@aBurmeseDev aBurmeseDev removed the needs-triage This issue or PR still needs to be triaged. label Jun 12, 2024
@aBurmeseDev aBurmeseDev self-assigned this Jun 12, 2024
@aBurmeseDev aBurmeseDev added response-requested Waiting on additional info and feedback. Will move to \"closing-soon\" in 7 days. p2 This is a standard priority issue labels Jun 12, 2024
@sazarkin
Copy link
Author

@aBurmeseDev was you able to run provided snippet? Let me know if you need additional guidance.

I am trying to understand how only importing SDK affects this html parsing code. This is not about slow import or slow api calls.

Maybe the sdk extends some base nodes classes? Maybe it does some modifications to runtime?

Are you aware of anything like that?

@github-actions github-actions bot removed the response-requested Waiting on additional info and feedback. Will move to \"closing-soon\" in 7 days. label Jun 13, 2024
@aBurmeseDev
Copy link
Member

aBurmeseDev commented Jun 13, 2024

@sazarkin - the code you shared is incomplete. It doesn't show where API call is being made or which operation is being called. It only shows that you're importing the client.

For example, here's CreateTable command page which is one of the client-dynamodb operations to create table, you'll see code example for CreateTable operation. You would first install client with your preferred method, import it and make the call.

Example Syntax
Use a bare-bones client and the command you need to make an API call.

Expand
Copy
import { DynamoDBClient, CreateTableCommand } from "@aws-sdk/client-dynamodb"; // ES Modules import

const client = new DynamoDBClient(config);
const input = { // CreateTableInput
  AttributeDefinitions: [ // AttributeDefinitions // required
    { // AttributeDefinition
      AttributeName: "STRING_VALUE", // required
      AttributeType: "S" || "N" || "B", // required
    },
  ],
  TableName: "STRING_VALUE", // required
  KeySchema: [ // KeySchema // required
    { // KeySchemaElement
      AttributeName: "STRING_VALUE", // required
      KeyType: "HASH" || "RANGE", // required
    },
  ],
  LocalSecondaryIndexes: [ // LocalSecondaryIndexList
    { // LocalSecondaryIndex
      IndexName: "STRING_VALUE", // required
      KeySchema: [ // required
        {
          AttributeName: "STRING_VALUE", // required
          KeyType: "HASH" || "RANGE", // required
        },
      ],
      Projection: { // Projection
        ProjectionType: "ALL" || "KEYS_ONLY" || "INCLUDE",
        NonKeyAttributes: [ // NonKeyAttributeNameList
          "STRING_VALUE",
        ],
      },
    },
  ],
  GlobalSecondaryIndexes: [ // GlobalSecondaryIndexList
    { // GlobalSecondaryIndex
      IndexName: "STRING_VALUE", // required
      KeySchema: [ // required
        {
          AttributeName: "STRING_VALUE", // required
          KeyType: "HASH" || "RANGE", // required
        },
      ],
      Projection: {
        ProjectionType: "ALL" || "KEYS_ONLY" || "INCLUDE",
        NonKeyAttributes: [
          "STRING_VALUE",
        ],
      },
      ProvisionedThroughput: { // ProvisionedThroughput
        ReadCapacityUnits: Number("long"), // required
        WriteCapacityUnits: Number("long"), // required
      },
      OnDemandThroughput: { // OnDemandThroughput
        MaxReadRequestUnits: Number("long"),
        MaxWriteRequestUnits: Number("long"),
      },
    },
  ],
  BillingMode: "PROVISIONED" || "PAY_PER_REQUEST",
  ProvisionedThroughput: {
    ReadCapacityUnits: Number("long"), // required
    WriteCapacityUnits: Number("long"), // required
  },
  StreamSpecification: { // StreamSpecification
    StreamEnabled: true || false, // required
    StreamViewType: "NEW_IMAGE" || "OLD_IMAGE" || "NEW_AND_OLD_IMAGES" || "KEYS_ONLY",
  },
  SSESpecification: { // SSESpecification
    Enabled: true || false,
    SSEType: "AES256" || "KMS",
    KMSMasterKeyId: "STRING_VALUE",
  },
  Tags: [ // TagList
    { // Tag
      Key: "STRING_VALUE", // required
      Value: "STRING_VALUE", // required
    },
  ],
  TableClass: "STANDARD" || "STANDARD_INFREQUENT_ACCESS",
  DeletionProtectionEnabled: true || false,
  ResourcePolicy: "STRING_VALUE",
  OnDemandThroughput: {
    MaxReadRequestUnits: Number("long"),
    MaxWriteRequestUnits: Number("long"),
  },
};
const command = new CreateTableCommand(input);
const response = await client.send(command);

Hope that makes sense, let me know if you have any other questions or if I'm missing something?

@aBurmeseDev aBurmeseDev added the response-requested Waiting on additional info and feedback. Will move to \"closing-soon\" in 7 days. label Jun 13, 2024
@sazarkin
Copy link
Author

@aBurmeseDev looks like we hitting a wall here. Please ignore all previous instructions.
Do you know what this version of SDK does on import compared to v2. Same code with only import of v2 performs as it should.

@sazarkin sazarkin changed the title AWS SDK DynamoDB affects perfomance of third party library Importing AWS SDK DynamoDB affects perfomance of third party library Jun 14, 2024
@sazarkin
Copy link
Author

Maybe let me show the steps:

  • Run provided script node script.js
  • It will show time it took: X ms
  • Run script with SDK import AWS_SDK=1 node script.js
  • I will show time it took: Y ms
  • Y is 15-20% higher than X
  • Conclusion: import aws-sdk v3 adds 15% overhead on string parsing

@trivikr trivikr changed the title Importing AWS SDK DynamoDB affects perfomance of third party library Importing AWS SDK DynamoDB affects performance of third party library Jun 14, 2024
@kuhe kuhe self-assigned this Jun 14, 2024
@kuhe
Copy link
Contributor

kuhe commented Jun 14, 2024

The SDK does not modify your unrelated library or global functionality.

I do notice the slowdown when dynamodb is imported. It could be triggering a compiler de-optimization below the JavaScript level.

It seems to be the import of @smithy/smithy-client.

@kuhe kuhe added the queued This issues is on the AWS team's backlog label Jun 14, 2024
@github-actions github-actions bot removed the response-requested Waiting on additional info and feedback. Will move to \"closing-soon\" in 7 days. label Jun 15, 2024
@kuhe
Copy link
Contributor

kuhe commented Dec 6, 2024

I isolated it to a call to

Object.create(String.prototype)

in https://github.com/smithy-lang/smithy-typescript/blob/main/packages/smithy-client/src/lazy-json.ts

I don't know why it would cause global de-optimization, since theoretically this is a read operation and it's not like the other library code uses the created class, but I will see if we can remove this.

@kuhe
Copy link
Contributor

kuhe commented Dec 6, 2024

I hypothesize the creation of an additional String-like class confuses the optimization of dispatching method calls on Strings themselves. This is what testing has indicated. I will still proceed with removing extension of String in the SDK.

It looks to be related to autoboxing of string primitives.

@kuhe kuhe changed the title Importing AWS SDK DynamoDB affects performance of third party library extending String de-optimizes method dispatch / Importing AWS SDK DynamoDB affects performance of third party library Dec 6, 2024
@kuhe kuhe added the pending-release This issue will be fixed by an approved PR that hasn't been released yet. label Dec 9, 2024
@kuhe
Copy link
Contributor

kuhe commented Dec 10, 2024

A fix was released in https://github.com/aws/aws-sdk-js-v3/releases/tag/v3.709.0.

The fix is available in v3.5.0 of @smithy/smithy-client, which should be compatible with older v3.x AWS SDK versions as well that take a range dependency of ^3.x on this package.

@kuhe kuhe added response-requested Waiting on additional info and feedback. Will move to \"closing-soon\" in 7 days. p1 This is a high priority issue and removed pending-release This issue will be fixed by an approved PR that hasn't been released yet. labels Dec 10, 2024
@kuhe kuhe removed the p2 This is a standard priority issue label Dec 10, 2024
@kuhe
Copy link
Contributor

kuhe commented Dec 12, 2024

Original PR adding the feature: #899

@github-actions github-actions bot removed the response-requested Waiting on additional info and feedback. Will move to \"closing-soon\" in 7 days. label Dec 13, 2024
@kuhe kuhe closed this as completed Dec 13, 2024
Copy link

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs and link to relevant comments in this thread.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 28, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug This issue is a bug. p1 This is a high priority issue queued This issues is on the AWS team's backlog
Projects
None yet
Development

No branches or pull requests

3 participants