search

h0x91b / redis-fast-driver

78 stars 13 forks source link

improve realloc performance #20

Closed adrian-gierakowski closed 6 years ago

adrian-gierakowski commented 6 years ago

avoid unnecessary copying when reallocating command buffer

this is a proper implementation of what was intended in 92b47664b20201eb95dc1526b83a97ae13179831

and a few other fixes, see individual commits for details

I am going to follow up with a few other commits shortly

adrian-gierakowski commented 6 years ago

the commits from ba8d73771958425cbcb21adee43d81e778bfb727 onward are about reducing number of reallocations see travis log for how many reallocations are needed with the current buffer growth strategy: https://travis-ci.org/h0x91b/redis-fast-driver/jobs/360343899

5bb94241222a8104fd0b20b74faee384267aa142 this is reduced to 1

h0x91b commented 6 years ago

1.2 strategy must be aligned by 64 bytes for a performance. And several small things, please see review.

adrian-gierakowski commented 6 years ago

Thanks for the quick review!

I'll try to take care of this tonight, although I'm leaving for holidays tomorrow morning and have a bunch of other things to do still so might run out of time.

Btw. would you mind sharing a link to the StackOverflow article you quoted?

On Sat, 31 Mar 2018, 10:13 Arseniy Pavlenko, notifications@github.com wrote:

Assigned #20 https://github.com/h0x91b/redis-fast-driver/pull/20 to @adrian-gierakowski https://github.com/adrian-gierakowski.

— You are receiving this because you were assigned.

Reply to this email directly, view it on GitHub https://github.com/h0x91b/redis-fast-driver/pull/20#event-1550669288, or mute the thread https://github.com/notifications/unsubscribe-auth/AAUJwZQJU2eLkL8TcAu20pCffav5UUQdks5tj0iegaJpZM4TB4_a .

adrian-gierakowski commented 6 years ago

@h0x91b I've done what you asked for, although I am not sure if rounding up to multiple of 64 actually improves anythings.

  1. if the purpose is to have memory page aligned the we should also adjust the pointer to point to start of the page, which we are not doing
  2. as this article suggests https://stackoverflow.com/a/3190185, it might be better to simply tell malloc exactly how much memory you need and let it figure out how to efficiently allocate it.

anyway, I guess no harm rounding it :)

adrian-gierakowski commented 6 years ago

@h0x91b, I believe I have implemented everything you've requested. Do you have any further doubts or comments?