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

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

Open
agentzh opened this Issue Jan 9, 2017 · 20 comments

Comments

Projects
None yet
8 participants
@agentzh
Member

agentzh commented Jan 9, 2017

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

This comment has been minimized.

Contributor

alubbe commented Jan 9, 2017

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

This comment has been minimized.

Member

agentzh commented Jan 9, 2017

@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

This comment has been minimized.

Member

thibaultcha commented Jan 9, 2017

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

This comment has been minimized.

Member

agentzh commented Jan 9, 2017

@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

This comment has been minimized.

Member

agentzh commented Jan 9, 2017

@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

This comment has been minimized.

Member

thibaultcha commented Jan 9, 2017

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

This comment has been minimized.

Member

agentzh commented Jan 9, 2017

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

This comment has been minimized.

Member

agentzh commented Jan 9, 2017

@thibaultcha Fair enough. I agree.

@p0pr0ck5

This comment has been minimized.

Contributor

p0pr0ck5 commented Jan 10, 2017

+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

This comment has been minimized.

nelson2005 commented Jan 10, 2017

@detailyang

This comment has been minimized.

Member

detailyang commented Jan 10, 2017

+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

This comment has been minimized.

bungle commented Jan 10, 2017

+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

This comment has been minimized.

nelson2005 commented Jan 10, 2017

@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

This comment has been minimized.

Member

agentzh commented Jan 11, 2017

@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

This comment has been minimized.

Member

agentzh commented Jan 11, 2017

@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

This comment has been minimized.

Member

agentzh commented Jan 11, 2017

@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

This comment has been minimized.

djczaski commented Jan 13, 2017

-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

This comment has been minimized.

Member

agentzh commented Jan 13, 2017

@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

This comment has been minimized.

Member

agentzh commented Jan 13, 2017

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

@nelson2005

This comment has been minimized.

nelson2005 commented Jan 13, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment