lua-nginx-module icon indicating copy to clipboard operation
lua-nginx-module copied to clipboard

Published 20 hours ago • verified publisher icon openresty

New Feature: added args support for ngx.on_abort

Open guanlan opened this issue 11 years ago • 11 comments

guanlan avatar Aug 13 '13 21:08 guanlan

In typical Lua usage, one would just pass a closure to on_abort rather than using pass values:

 ngx.on_abort(function() someotherfunction(my, arguments, for, the, other, function) end )

bakins avatar Aug 13 '13 22:08 bakins

Because we need keep consistent with ngx.timer.at API.

guanlan avatar Aug 13 '13 22:08 guanlan

@bakins Creating new closures at every request does introduce some overhead (observable in on-CPU time Flame Graphs, for example), so I think we need the ability to pass user arguments just as in ngx.timer.at().

agentzh avatar Aug 13 '13 22:08 agentzh

@agentzh I'm not surprised by the creation of a closure on each request showing up in a flame graph, I just wonder how much difference it makes in the real world. I don't have any strong feelings either way other than using a closure is more "Lua like."

bakins avatar Aug 16 '13 22:08 bakins

@bakins Creating closures per request could be a real bottleneck, as observed as 20% ~ 30% overall overhead in an on-CPU time flame graph for our Lua WAF system. Even though in that case it was not ngx.on_abort() in particular, but more aggressive use of "higher-order functions".

agentzh avatar Aug 19 '13 18:08 agentzh

This pull request is now in conflict :(

mergify[bot] avatar Jun 26 '20 00:06 mergify[bot]