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

Improve cookie baking performance #1

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jumper047
Copy link

@jumper047 jumper047 commented Sep 6, 2023

String concatenation in lua is not very performant (see this question on SO). I tried to improve performance of the library and changed concatenation to table:concat. Performance on my usecase (to set cookie on every incoming request) improved from 215000 to 220000 requests per second.

@utix
Copy link
Owner

utix commented Oct 19, 2023

Can you share a benchmark test ?
I think we can rework your commit to have a loop I want to check if the perf improvement is still good

@jumper047
Copy link
Author

Sorry, but I can't - these results were obtained on test server in company I work. But I can test it again with reworked commit. Test itself is, actually, was pretty simple - Yandex Tank to generate load, and nginx with plugin, that generates some cookie and sets it with lua-resty-cookie. First result (215k rps) was obtained with package from luarepo, and second (220k) with my patch. I ran multiple tests (at least 5 for each case) to be sure this is not a coincidence.

@bungle
Copy link

bungle commented Feb 5, 2024

String buffer could be even faster:
https://luajit.org/ext_buffer.html

Though I think LuaJIT should (?) be able to compile the string concatenation above, but not sure as it is a bit more complicated than straight string .. string concatenation.

utix pushed a commit that referenced this pull request Jul 22, 2024
Support for multiple values with the same key
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

Successfully merging this pull request may close these issues.

3 participants