-
Notifications
You must be signed in to change notification settings - Fork 2k
New Feature: added args support for ngx.on_abort #269
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
base: master
Are you sure you want to change the base?
Conversation
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 ) |
Because we need keep consistent with ngx.timer.at API. |
@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 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 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". |
e7ac10c
to
cfd4f90
Compare
This pull request is now in conflict :( |
f924579
to
fef2581
Compare
This pull request is now in conflict :( |
This pull request is now in conflict :( |
This pull request is now in conflict :( |
This pull request is now in conflict :( |
This pull request is now in conflict :( |
This pull request is now in conflict :( |
This pull request is now in conflict :( |
No description provided.