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

perf(NODE-4635): migrate deque to js-sdsl #3419

Closed
wants to merge 2 commits into from

Conversation

ZLY201
Copy link

@ZLY201 ZLY201 commented Sep 16, 2022

Description

What is changing?

Migrate deque fromdenque to js-sdsl.

Is there new documentation needed for these changes?

Yes, because I changed deque's source.

What is the motivation for this change?

Hey! I'm the developer of Js-sdsl. Official website: https://js-sdsl.github.io/.

Now, we published the version 4.1.4.

I see you are using denque.

In benchmark, we have confirmed that Js-sdsl is several times faster than denque and nearly equal to Array.push in the case of push elements.

We would like to invite you to migrate deque related functions to Js-sdsl v4.1.4.

Here are the benmarks:

  • main

https://github.com/ZLY201/node-mongodb-native/actions/runs/3060269558/jobs/4938575049

{
  microBench: {
    bsonBench: {
      flatBsonEncoding: 94.25118920101988,
      flatBsonDecoding: 82.87358886201503,
      deepBsonEncoding: 25.001962953249738,
      deepBsonDecoding: 24.653867111950653,
      fullBsonEncoding: 50.021585046736845,
      fullBsonDecoding: 55.68924843838592
    },
    singleBench: {
      runCommand: 0.040064720730158744,
      findOne: 2.5369494102430736,
      smallDocInsertOne: 0.528170445690162,
      largeDocInsertOne: 13.681842865860268
    },
    multiBench: {
      findManyAndEmptyCursor: 33.56057421596245,
      smallDocBulkInsert: 13.402725334538117,
      largeDocBulkInsert: 16.841357236728616,
      gridFsUpload: 255.17269063702042,
      gridFsDownload: 480.75867257973243
    }
  },
  bsonBench: 55.41524026889301,
  singleBench: 5.582320907264502,
  multiBench: 159.9472040007964,
  parallelBench: NaN,
  readBench: 172.28539873531267,
  writeBench: 59.925357303967516,
  driverBench: 116.10537801964009,
  bsonType: 'js-bson'
}

 

  • perf/migrate-deque

https://github.com/ZLY201/node-mongodb-native/actions/runs/3060363980/jobs/4938787566

{
  microBench: {
    bsonBench: {
      flatBsonEncoding: 116.1402016879238,
      flatBsonDecoding: 105.61033779223237,
      deepBsonEncoding: 30.501721686298122,
      deepBsonDecoding: 31.369037681697147,
      fullBsonEncoding: 58.73530768775185,
      fullBsonDecoding: 76.3827782555276
    },
    singleBench: {
      runCommand: 0.06605681122165877,
      findOne: 4.00111914410605,
      smallDocInsertOne: 0.820494770454103,
      largeDocInsertOne: 18.72604194153827
    },
    multiBench: {
      findManyAndEmptyCursor: 45.29806933931308,
      smallDocBulkInsert: 16.440118643374323,
      largeDocBulkInsert: 22.128555133680806,
      gridFsUpload: 400.0434088682383,
      gridFsDownload: 702.5095912389725
    }
  },
  bsonBench: 69.78989746523848,
  singleBench: 7.849218618699474,
  multiBench: 237.2839486447158,
  parallelBench: NaN,
  readBench: 250.60292657413052,
  writeBench: 91.63172387145717,
  driverBench: 171.11732522279385,
  bsonType: 'js-bson'
}

You can see all the results are improved.

Double check the following

  • Ran npm run check:lint script
  • Self-review completed using the steps outlined here
  • PR title follows the correct format: <type>(NODE-xxxx)<!>: <description>
  • Changes are covered by tests
  • New TODOs have a related JIRA ticket

@ZLY201 ZLY201 force-pushed the perf/migrate-deque branch 3 times, most recently from ad6a24e to 6f54ec5 Compare September 16, 2022 05:14
@bajanam bajanam added the External Submission PR submitted from outside the team label Sep 16, 2022
@baileympearson
Copy link
Contributor

@ZLY201 do you have memory usage benchmarks comparing js-sdsl to denque?

@ZLY201
Copy link
Author

ZLY201 commented Sep 16, 2022

@ZLY201 do you have memory usage benchmarks comparing js-sdsl to denque?

What kind of test do you mean?

Maybe build a new benchmark based on mongodb or just external simple tests between js-sdsl and deque?

}
--index;
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here can be optimized.

The time complexity is O(n^2) now.

I'll make it O(n) later.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please check it @baileympearson

@ZLY201
Copy link
Author

ZLY201 commented Sep 17, 2022

OK, I done a simple memory test. See result in https://js-sdsl.github.io/#/test/benchmark-result?id=deque.

And test action is here: https://github.com/js-sdsl/benchmark/actions/runs/3074028042/jobs/4966563253.

You can see when the size is smaller than 10^3 we will use 30KB more memory than denque because we reserve (1 << 12) size for the container in the constructor of Deque.

You can change this number by use new Deque([], 1 << 10).

And when the size be larger, Deque are saving more and more memory because memory is dynamically allocated in Deque.

Combined with previous performance tests, I think this migrate will make a significant improvement. :)

@baileympearson

@nbbeeken nbbeeken added the tracked-in-jira Ticket filed in MongoDB's Jira system label Sep 20, 2022
@nbbeeken
Copy link
Contributor

Hi @ZLY201, thanks for your effort here, at this time we're not prepared to adopt this change we have a project planned that's going to deep dive into our performance issues and identify how we can better measure our JS specific performance concerns. You may notice all the benchmarks currently relate to performing networking with MongoDB and only output measurements in MS/s. With that work we'll be better equipped to make data informed decisions about changing our internals to be more performant. I've moved the related ticket to our backlog but attached it to our to the perf project where we'll be taking a closer look at this. Thanks again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
External Submission PR submitted from outside the team tracked-in-jira Ticket filed in MongoDB's Jira system
Projects
None yet
4 participants