feature: support pass table to send_query #68
Conversation
lib/resty/mysql.lua
Outdated
| return #a | ||
| end | ||
| local n = 0 | ||
| for i, v in pairs(a) do |
There was a problem hiding this comment.
@yanxurui Typo? Maybe you should use ipairs() here instead? The pairs() loop cannot be JIT compiled.
There was a problem hiding this comment.
Sorry, I thought pairs can include non-numeric indexed items in a table but it's wrong because concat or cosocket:send will ignore non-numeric items.
lib/resty/mysql.lua
Outdated
| local function _send_packet(self, req, size) | ||
| local sock = self.sock | ||
|
|
||
| self.packet_no = self.packet_no + 1 | ||
|
|
||
| -- print("packet no: ", self.packet_no) | ||
|
|
||
| local packet = _set_byte3(size) .. strchar(band(self.packet_no, 255)) .. req | ||
| local packet = {_set_byte3(size), strchar(band(self.packet_no, 255)), req} |
There was a problem hiding this comment.
One sad thing is this extra allocation of a new Lua temp table. Maybe we can reuse this table across different function calls?
There was a problem hiding this comment.
You are right.
I put the table in the module level instead of instance because multiple instances can share it.
This looks dirty but I can's find a better way.
lib/resty/mysql.lua
Outdated
|
|
||
| local bytes, err = _send_packet(self, cmd_packet, packet_len) | ||
| local bytes, err = _send_packet(self, {strchar(COM_QUERY), query}, packet_len) |
| local function _concat_rec(t, sep) | ||
| for i, v in ipairs(t) do | ||
| if type(v) == "table" then | ||
| t[i] = _concat_rec(v, sep) |
There was a problem hiding this comment.
Maybe we can avoid this recursive concat call?
There was a problem hiding this comment.
If you mean change recursive to iterative? I don'k know which performs well but surely recursive method is more intuitive and requires less code.
I think this function doesn't matter because it's only called when debug.
|
@yanxurui Are you using recent versions of OpenResty? Recent versions of OpenResty use our own branch of LuaJIT with special optimizations which significantly reduce the hash collisions in Lua string table on Intel CPUs with SSE 4.2 support. I suggest you try it out, for example, the latest 1.11.3.6.1 RC1 release below: https://openresty.org/download/openresty-1.13.6.1rc1.tar.gz But sure, reducing the number of |
|
@agentzh It seems there is no need to avoid string creation in lua land. So this pull request may be not necessary. Thank you very much for your help. |
|
@yanxurui I'm glad that the latest OpenResty addresses your performance issue. I think I'd like to leave this PR as is for now. We do have plans to rewrite this library (as well as other |
|
Thank you for your awesome job in openresty community. |
problem
Luajit is poor at creating a lot of similar strings due to hash collisions. The discussion can be seen in Github: Reduce string hash collisions.
So it consumes a lot cpu when construct sql statements dynamically.
This problem has already been asked on openresty-cn's google group:
solution
Fortunately, cosocket's send method can receive a table of strings and it will handle it in C land. A temporary solution is to avoid string creation in lua and pass the table of sql segments into cosocket's send method just like resty-redis does.
result
In my application, the performance(QPS) improves 50%.
The flame graph proves that the lj_new_str is no longger the bottleneck:
Before:
After:
I hopes this can help others.