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

Migrate deque from denque to Js-sdsl Deque #209

Closed
ZLY201 opened this issue Sep 14, 2022 · 6 comments
Closed

Migrate deque from denque to Js-sdsl Deque #209

ZLY201 opened this issue Sep 14, 2022 · 6 comments

Comments

@ZLY201
Copy link

ZLY201 commented Sep 14, 2022

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 and I am willing to submit a pull request for this change.

Looking forward to your reply! :D

@rusher
Copy link
Collaborator

rusher commented Jan 12, 2023

Thanks for indicating that, but in our specific use-case, our benchmark performs better using denque: most of our use is push and shift and even if js-sdsl is faster in other things, denque performs better in this use-case
tested with :

const Benchmark = require('benchmark');
const { LinkList, Queue } = require('js-sdsl');
const QueueDenque = require('denque');

const t1 = new LinkList();
const t2 = new QueueDenque();
const t3 = new Queue(new Array(2000));

for (let i = 0; i < 600; i++) {
  t1.pushBack('elm');
  t2.push('elm');
  t3.push('elm');
}

var suite = new Benchmark.Suite();

// add tests
suite
  .add('LinkList with 1000 element push and shift', function () {
    t1.pushBack('elm');
    return t1.popFront();
  })
  .add('denque with 1000 element push and shift', function () {
    t2.push('elm');
    return t2.shift();
  })
  .add('Queue with 1000 element push and shift', function () {
    t3.push('elm');
    return t3.pop();
  })
  // add listeners
  .on('cycle', function (event) {
    console.log(String(event.target));
  })
  // run async
  .run({ async: true });

resulting in

LinkList with 1000 element push and front x 10,151,006 ops/sec ±3.79% (68 runs sampled)
denque with 1000 element push and shift x 171,003,836 ops/sec ±0.56% (94 runs sampled)
Queue with 1000 element push and shift x 19,941,066 ops/sec ±1.07% (95 runs sampled)

@ZLY201
Copy link
Author

ZLY201 commented Jan 13, 2023

Thanks for your reply!

But I think this test cannot reflect the real performance difference between the two structures. The reasons as follow:

  1. You should use Deque but not Queue in js-sdsl. Queue is a little slow than Deque.
  2. The performance bottleneck of the data structure named double-ended-queue are reflected in capacity expansion. I submitted Add benchmark with Js-sdsl invertase/denque#49 but no reply.
  3. This test method is just a simple simulation and does not really reflect its performance in the database connection. I am looking for a way to test it more realistically. Find a way to do bench for database connection js-sdsl/js-sdsl#124

In summary, I think it is unreasonable to think that the performance of js-sdsl is worse then denque.

@rusher
Copy link
Collaborator

rusher commented Jan 13, 2023

I didn't know about queue beeing slower than deque, i would have think the opposite.

In fact, I've done this benchmark because i've first made the change to js-sdsl in development, and run the db benchmarks, resulting in worst performance. After analysis, this was due to this scenario "push and shift" that is slower in js-sdsl.

I'll have to redo my bench with deque in place of Queue :)

@ZLY201
Copy link
Author

ZLY201 commented Jan 16, 2023

Sorry, I found the reason for its slowness. This was caused by an PR which want to fixes elements not being empty.

I'll fix it later.

@ZLY201
Copy link
Author

ZLY201 commented Jan 20, 2023

The version 4.3.0 has been published. You can try it again. @rusher

@rusher
Copy link
Collaborator

rusher commented Feb 8, 2023

I've tryed again, and even if 4.3.0 perform better, denque still fit better in connector use (in fact implementation fit exactly to use, it will be hard to replace).
So i'm closing this issue

@rusher rusher closed this as completed Feb 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants