openresty / lua-nginx-module

Embed the Power of Lua into NGINX HTTP servers
https://openresty.org/
11.34k stars 2.04k forks source link

Switch over to lua-resty-core and retire old CFunction-based Lua API impl #949

Open agentzh opened 7 years ago

agentzh commented 7 years ago

As originally discussed in #942, I'd like to discuss the option of forcibly switching over to lua-resty-core in this ngx_http_lua module and drop the existing CFunction-based Lua API implementation in the current ngx_http_lua module core.

The upside of this change is that we no longer have to maintain two parallel implementations of the same Lua API in both ngx_http_lua_module and lua-resty-core. And the ngx_http_lua_module core will be much smaller in terms of code size.

I propose the new lua_use_resty_core configuration directive for fine controls. By default, the resty.core module is automatically loaded right before running the init_by_lua* hook (if any). The user can avoid that by configuring the following directive in her nginx.conf:

lua_use_resty_core none;

Furthermore, the user can choose to load a selectively subset of submodules of resty.core.* like below:

# only load modules resty.core.regex, resty.core.shdict, and resty.core.req
lua_use_resty_core regex shdict req;

This leads to even a little bit smaller runtime memory footprint and less GC overhead (across those API function objects and containing Lua tables).

Or just load everything:

lua_use_resty_core all;

And loading everything under the resty.core.* namespace is the default.

A natural consequence of this change is that we also drop the long-time support of the standard Lua 5.1 interpreter in ngx_http_lua and the whole OpenResty. LuaJIT is already diverged a lot from it and it's becoming a big burden for us due to the lack of FFI in the standard Lua 5.1 interpreter, for example.

Another natural consequence of this is that ngx_http_lua_module will also depend on lua-resty-lrucache which is used by lua-resty-core's resty.core.regex submodule (for the compiled regex cache used by the ngx.re.* API).

alubbe commented 7 years ago

Very much in favor of this.

Couple of points of that occured to me. Does this mean that a) all "old" functionality will be ported to the ffi interface (esp. sockets)? b) until then, the old interfaces that do not have an ffi-equivalent get bug fixes but no new features? c) all new features will be ffi-only?

agentzh commented 7 years ago

@alubbe To answer your questions:

  1. That's always the plan. We welcome community contributions and it's being done piece by piece. Yes, FFI-based ngx.print/ngx.say/cosockets are always on the roadmap.
  2. We only retire CFunction-based impl that has FFI equivalents in lua-resty-core. Those remain will still receive both bug fixes and new features as always.
  3. Yes, definitely. And also in proper ngx.blah Lua modules like ngx.re and ngx.semaphore instead of inserting into the global giant ngx table.
thibaultcha commented 7 years ago

A paste of the reply I was writing in #942:

All of this makes a lot of sense to me. I had a hunch that Lua 5.1 would not last long if we went down that path indeed :)

I believe that being stuck to Lua 5.1 does not serve the community so well anyways; it would eventually make sense to upgrade to Lua 5.3 and drop LuaJIT, or stick to LuaJIT only, and the later of course makes a lot more sense in our case. The former is not an option for obvious reasons.

If I recall correctly, some users are stuck to PUC-Lua for platform support reasons. It would make sense to gather those use cases before such a drastic change, to be aware of the impacts that decision would have.

The lua_use_resty_core is I believe a powerful approach, I am very much in favor of a way to selectively load a set of submodules from resty.core.

agentzh commented 7 years ago

@thibaultcha Well, I think we can keep a branch of ngx_http_lua_module with all those CFunction-based impl. And this branch will only receive important bug fixes. But no releases will be made out of it nor any new features. Does that sound good enough?

agentzh commented 7 years ago

@thibaultcha But it still adds maintenance overhead on our side :) So I'm afraid we'll need paid users or long-term volunteers to justify this extra effort :)

thibaultcha commented 7 years ago

And this branch will only receive important bug fixes. But no releases will be made out of it nor any new features.

Sounds like a plan. Basically this branch would be "deprecated" and only advertised to Lua 5.1 users, and updated in the only event that an important bug fix is required. Such users can stick to existing OpenResty releases, but in the event a bugfix is applied to that branch, they might need to manually re-compile OpenResty (since no more official OR releases would be provided with 5.1 support).

I believe this to be reasonable but am simply stating it out loud to raise awareness for future reference :)

agentzh commented 7 years ago

If we go this route, the Big Change will happen after the new OpenResty 1.11.8.1 release is out to the earliest.

agentzh commented 7 years ago

@thibaultcha Fair enough. I agree.

p0pr0ck5 commented 7 years ago

+1 from me. In the past I've had to selectively load resty.core pieces to avoid bugs, so doing this natively would certainly be nice.

TBH I don't even know if maintaining a "legacy" branch would be worth it, at least in the long term- that pattern never works well. At some point I imagine the codebases would diverge to the point that maintenance would be a nightmare. And at some point the cord should be cut.

nelson2005 commented 7 years ago

+1 from me as well.  It's unintuitive to me that the recommended configuration requires extra work, which beginners are likely to miss and thereby build new legacy code.My recommendation is that the openresty work in the current best-practices way.  If someone needs the previous behavior, maybe that could be specified in the config (as long as it's practical to maintain)?On Jan 9, 2017 8:50 PM, Robert notifications@github.com wrote:+1 from me. In the past I've had to selectively load resty.core pieces to avoid bugs, so doing this natively would certainly be nice. TBH I don't even know if maintaining a "legacy" branch would be worth it, at least in the long term- that pattern never works well. At some point I imagine the codebases would diverge to the point that maintenance would be a nightmare. And at some point the cord should be cut.

—You are receiving this because you are subscribed to this thread.Reply to this email directly, view it on GitHub, or mute the thread.

detailyang commented 7 years ago

+1 for forcibly switching over to lua-resty-core and let contributors be more focused to develop new feature:)

we no longer have to maintain two parallel implementations of the same Lua API in both ngx_http_lua_module and lua-resty-core

bungle commented 7 years ago

+1

Couple of points:

  1. OpenResty as a ecosystem is already heavily dependent on LuaJIT and FFI.
  2. LuaJIT is already pretty much not following PUC-Lua.
  3. OpenResty is not fully compatible with PUC-Lua ecosystem.

Against the change:

  1. PUC-Lua runs (everywhere) on places where LuaJIT doesn't or where LuaJIT is buggy.
  2. PUC-Lua has a more certain future compared to LuaJIT's (Mike Pall's resignation, pull-requests going nowhere or marked as won't fix, no replacement for Mike Pall, etc.).
  3. PUC-Lua has evolved as a language whereas LuaJIT has not.

OpenResty seems to be evolved in steps:

  1. Nginx + Modules
  2. Nginx + Modules + Lua Module
  3. Nginx + Modules + Lua Module + FFI
  4. Nginx + Modules + Lua Module + FFI + Lua Libraries

Each step has changed the OpenResty's direction a little bit, and sometimes quite fundamental ways. For example, I don't think many of the OpenResty's original Nginx Modules are that important anymore as they have got equivalent and more flexible higher level implementations as scriptable Lua libraries. At some point we could discuss about deprecating those modules that do have a better and more up to date Lua library.

I do think that we should provide a release where we warn the users that some features will be removed in a next release, and document the migration paths. We should keep a branch before this breaking change in security updates only mode for one year or so(?).

lua-resty-lrucache should be merged to core to avoid the core having external dependencies.

I think this change has many benefits.

nelson2005 commented 7 years ago

@bungle- the LuaJIT lack of path forward is probably the most concerning point for the corporate world. Introducing unmaintained software is a hard sell. Not that I have any solutions.

agentzh commented 7 years ago

@bungle @nelson2005 I'm starting the company, OpenResty Inc., which will invest a lot on LuaJIT and support talented engineers to push LuaJIT forward :) Our nonprofit organization, OpenResty Software Foundation, will do the same, of course. Oh, we already have a concrete roadmap for this year's LuaJIT development :)

LuaJIT is already a corner stone of the OpenResty world that we will keep standing on :)

agentzh commented 7 years ago

@bungle @nelson2005 We've already implemented a good subset of the Perl 6 language in a true compiler targeting OpenResty/LuaJIT and the result is very promising. I think other scripting languages like PHP, Python, Ruby, and JavaScript will follow soon. The plan is to also enhance and consolidate LuaJIT to serve as a kind of "common language runtime" for dynamic languages atop OpenResty. It's a bit OT but my point is that there's no way for us to let LuaJIT go nowhere :)

agentzh commented 7 years ago

@nelson2005 Also, I think it's very unfair to call a software whose official git repository always keeps receiving fresh commits "unmaintained":

https://github.com/LuaJIT/LuaJIT/commits/v2.1

And new OpenResty releases in the past year keep bringing in new official LuaJIT fixes and features, as can be seen from the official release announcements and change logs.

djczaski commented 7 years ago

-1 I would rather ngx_lua continue to support both Lua and LuaJIT. We have a number of products that use ngx_lua without any extra OpenResty pieces or LuaJIT.

agentzh commented 7 years ago

@djczaski Seems like a good opportunity to migrate to OpenResty? ;) The official nginx core has long-standing known issues and limitations and never fully pass ngx_lua's test suite BTW.

agentzh commented 7 years ago

@djczaski Also, the Lua API provided by lua-resty-core is usually much faster than ngx_lua core's current CFunction-based API.

nelson2005 commented 7 years ago

Thanks, that gives me some comfort!On Jan 11, 2017 1:29 PM, Yichun Zhang notifications@github.com wrote:@bungle @nelson2005 We've already implemented a good subset of the Perl 6 language atop OpenResty/LuaJIT and the result is very promising. I think other scripting languages like PHP, Python, Ruby, and JavaScript will follow soon. The plan is to also enhance and consolidate LuaJIT to serve as a kind of "common language runtime" for dynamic languages. It's a bit OT but my point is that there's no way for us to let LuaJIT go nowhere :)

—You are receiving this because you were mentioned.Reply to this email directly, view it on GitHub, or mute the thread.

snizovtsev commented 3 years ago

I like the idea of having unified API style across the project. But for some cases it could be sub optimal.

Luajit FFI is designed to work with native C APIs which have no knowledge on Lua. It require all arguments to be convertible to/from C types. This means that Lua structures couldn't be stashed for later use with luaL_ref(L, LUA_REGISTRYINDEX).

That style of APIs are used to implement callback arguments like ngx.timer.at do. And while Luajit FFI can generate callback by its C prototype, its not clear how to manage lifetime of such closure.

I faced that while trying to extend an existing FFI based module with a callback argument.