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

Use constant array allocation for Socket buffer #28

Open
ysa23 opened this issue Feb 3, 2022 · 3 comments
Open

Use constant array allocation for Socket buffer #28

ysa23 opened this issue Feb 3, 2022 · 3 comments

Comments

@ysa23
Copy link
Owner

ysa23 commented Feb 3, 2022

Instead of dynamically allocating a new array on each flush, create an array with constant size, and use it with a pointer to add new reports.
The goal is to reduce CPU strain - since pushing to an array might cause it to be reallocated in memory, and can invoke GC.

On the initialization add an additional optional parameter which contain the size to allocate.
Buffer should be flushed when either buffer size is exceeded, array size is exceeded or on interval.

@ysa23 ysa23 added the enhancement New feature or request label Feb 3, 2022
@ysa23 ysa23 added tech enhancement and removed enhancement New feature or request labels Feb 18, 2022
@metcoder95
Copy link

What are your initial thoughts on the approach to this?

I made some benchmarks to test the hypothesis and so far performance will be gained.
Though the problem might not shift/pop pieces of data to not alter the size of the Array, but rather use some sort of mark & sweep approach so it can flag its time to flush and new spaces are ready to be filled. I'm thinking of a Ring Buffer here maybe.

Benchmarks:

Code:

const Benchmark = require('benchmark')

const suite = new Benchmark.Suite()

const totalLength = 100
const values = Array.from({ length: totalLength }).map((_, index) => index + 1)

suite
  .add('Dynamic', function () {
    let arr = []
    for (let i = 0; i < totalLength; i++) {
      arr.push(values[i])
    }
  })
  .add('Constant', function () {
    let arr = new Array(totalLength)
    for (let i = 0; i < totalLength; i++) {
      arr[i] = values[i]
    }
  })
  .on('cycle', function (event) {
    console.log(String(event.target))
  })
  .on('complete', function () {
    console.log('Fastest is ' + this.filter('fastest').map('name'))
  })
  .run({ async: true })

image

@ysa23
Copy link
Owner Author

ysa23 commented Feb 20, 2023

What are your initial thoughts on the approach to this?

I made some benchmarks to test the hypothesis and so far performance will be gained. Though the problem might not shift/pop pieces of data to not alter the size of the Array, but rather use some sort of mark & sweep approach so it can flag its time to flush and new spaces are ready to be filled. I'm thinking of a Ring Buffer here maybe.

Benchmarks:

Code:

const Benchmark = require('benchmark')

const suite = new Benchmark.Suite()

const totalLength = 100
const values = Array.from({ length: totalLength }).map((_, index) => index + 1)

suite
  .add('Dynamic', function () {
    let arr = []
    for (let i = 0; i < totalLength; i++) {
      arr.push(values[i])
    }
  })
  .add('Constant', function () {
    let arr = new Array(totalLength)
    for (let i = 0; i < totalLength; i++) {
      arr[i] = values[i]
    }
  })
  .on('cycle', function (event) {
    console.log(String(event.target))
  })
  .on('complete', function () {
    console.log('Fastest is ' + this.filter('fastest').map('name'))
  })
  .run({ async: true })

image

There are two separate issues to consider:

  1. Remove push and pop in favor of constant solution - both mark & sweep and ringed buffer will work - thanks for the suggestion.
  2. New array allocation after each send - my thoughts here are instead of creating a new array, simply "empty" the existing one once the buffer has been serialized.

The new line concat is also an easy win - so we won't need to create another array during send (src/socket.js)

Probably need to split this issue into these three tasks

@metcoder95
Copy link

New array allocation after each send - my thoughts here are instead of creating a new array, simply "empty" the existing one once the buffer has been serialized.

Totally. It comes hand in hand with the Ring Buffer or Mark & Sweep one. With Ring Buffer it can be to flush it every X amount of seconds or just when a threshold has been reached (Ring Buffer plays almost perfectly in both, as you can concurrently or almost parallel flush and still receive incoming chunks).

The new line concat is also an easy win - so we won't need to create another array during send (src/socket.js)

Would be interesting to test

Probably need to split this issue into these three tasks

Sounds like a good plan. Mostly as two optimization topics are being handled, plus management of the buffering that might change slightly the current implementation and will require some time 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants