Add support for using doubles in LUA53 build

[pjsg]

In order to improve compatibility with the LUA51 build, it would be very useful to have support for doubles in LUA53. Doubles are essential for storing times since the epoch if you want better than 1 second precision.

[nwf]

In order to keep TVal 8 bytes, we would have to resort to something like NaN packing, which I don't think Lua does? Not saying it's impossible, but I think it'd take some engineering.

Given that we intend float+int32 to be the default configuration, it would, I think, be better if our APIs for time keeping were split into 32-bit pairs (e.g., seconds * nanoseconds), even if that's slightly more complicated for application developers.

[TerryE]

Because the Lua 5.3 engine does int calculations using the default int size, I suspect that we will have just as many issues with int32 + double, IMO. I think that we would need to look at int64 + double which is going to have significant runtime implications.

If you want to exploit the property that denormalised doubles to store 54 bit integer values, then you really need to stick with the double-only Lua 5.1 ecosystem.

Standard Lua offers various time support functions in os: date(), time() and difftime(). @jmattsson suggested when we had this debate maybe 9 months ago and agreed on the float + int32 default that we might consider some extended arithmetic helper functions. We could debate various approaches, but I like @nwf 's idea of secs + nanosecs either as a list pair or an array. It would be straightforward adding a difftime() API call which encapsulate of the nastiness and with an optional argument do automatic scaling to secs, msecs or nsecs.

[TerryE]

How about node.difftime() returning a ftoat (6 d.p.) answer ? So

> difftime, int = node.difftime, math.floor
> = difftime(1594994427,0,1594994426,500000) -- list form
0.5
> = difftime({1594994427,0},{1594994426,500000}) -- array[2] form
0.5
> = difftime({sec = 1594994427},{sec =1594994426,usec=500000}) -- keyed array form note fields default to 0
0.5
-- answer is subtype type number so if you want (int) mSec for example
> a,b = {1594994427,0},{1594994426,500000}
> =int(difftime(a,b)*1000)
500

Orders of magnitude easier than trying to back engineer Lua 5.1 style arithmetic into the Lua 5.3 engine.

[pjsg]

The lua53 core supports 9 combinations of integer size and floating point size -- although I think that the ESP really only support 4 of those combinations (32/64 ints and float/double). One of those combinations was chosen for the lua53 version of nodemcu.

The PR adds support for a second combination of integer and float sizes. More importantly, this version is more compatible with the lua51 version of nodemcu.

As I understand it, we want to deprecate the lua51 build and move to only supporting the lua53 version. This will be a fundamental breaking change to some set of apps. Until we disable the lua51 build we are supporting the following:

• 32-bit Integer only build
• double only build
• 32-bit integer with float build.

The PR to enable

• 32-bit integer with double build

is tiny.

I am intrigued by the possibility of adding

• 64-bit integer with double == this is the default configuration for "big system" lua.

I suspect that this would require some pretty serious work to make sure that we were using lua_Integer everywhere instead of various int32_t flavors. However, most of our added modules work with the 32-bit integer build so it is probably only some places which would work (sjson, struct, etc).

[pjsg]

The problem with that definition of difftime is that it doesn't work for the lua51 integral builds. Yes, it is a pain that we have all of these flavors of int and float/double, but that is the current state.

I don't want nodemcu to have the same set of issues that python did in the 2 -> 3 transition where there was a fair amount of backwards incompatibility. It took them eleven years to stop supporting python 2 after python 3 came out. I really don't want to be supporting the lua51 code until 2030.

[TerryE]

I agree that the PR is tiny, though we will need to have test coverage of the paths it introduces, My issue is that a Lua 5.3 "32-bit integer with double build" will have different behaviour to our Lua 5.1 build. In the 5.1 integer expressions numerically smaller than 2^54 (IIRC) are exact thanks to IEEE 64bit f.p. behaviour. In 5.3 int expressions work just like the C int type so under and oveflow the same way.

=2147483647+1
-2147483648

This is still true with 64-bit floats, you would need to do something like (assuming everything else works):

=2147483647.0+1
2147483648.0

If people need to the old behaviours then we should just keep a (frozen) support for the (current and frozen) release Lua 5.1

[pjsg]

Good point. I think that that argues for having a 64-bit int / double build option. I'll take a look at that to see how much breaks....

[jmd13391]

we want to deprecate the lua51 build and move to only supporting the lua53 version.

AMEN TO THAT!!! Put a freeze stake in the LUA51 firmware and move on. So much time and effort has been invested in carrying the LUA51 baggage into the LUA53 firmware that it makes my blood pressure spike just reading the reams of threads on the topic (and I'm just a developer) You guys are trying to fit a LUA51 square peg in a LUA53 round hole. Let it go... LOL

[TerryE]

@pjsg Philip, we're adding over a dozen issues and PRs which are all bottom up facets of the same problem; one that was discussed over 6 months ago and we had reached a nominal conclusion. I accept that we need to revisit this if one of active committers has genuine concerns, but having this discussion randomly over so many issues and PRs threads makes this impossible to track for other committers. IMO, there is absolutely no point in creating point PRs for fragments of an architectural issue until that issue is resolved; yes we know that this code is changeable, but we can't change fragments in isolation without screwing up the integrity of the whole, so can please hold off an creating these until the core issue is first resolved.

Can we just discuss and resolve this core issue in one place. Rather than block your thread, I have done this on my #3220 (which is really about the same issue) and I suggest that we have this discussion here.

We seem to have three build options:

1. int32 + float32. Native int size. Best storage utilisation. Standard Lua build variant. Needs robust time abstraction.
2. int32 + float64. Native int size. Not a standard Lua build variant. Has gotchas when doing mixed int/float manipulation.
3. int64 + float64. Not a native int size. Standard Lua build variant. Storage and runtime overheads.

And we mustn't forget the current option (0) -- the Lua 5.1 build.

IMO supporting one option would be best and we had chosen (1) until you reopened this, so I suggest that if we are going to have a second then we need to have a debate about the pros and cons of 2 vs 3 vs neither.

I really thing that the complexity isn't the Lua core which is largely configure to support either a 32 bit or 64 bit build, but that ensuring that all of our libraries compile and work against (0), (1) and now {(2) or (3)}.

The issue that you raise that is valid against (1) is that we should have a robust encapsulation for handling epoch times down milli or nanosecond and the requires 52 bit precision. This encapsulation should work consistently across those library modules that need to support it, and also against all of our supported build variants -- (0), (1), (3) for example. Wherever practical we need to implement and encapsulate the abstraction as a set a macros that can be used across the relevant library modules and that will automatically generate the correct compiled code for the selected build variant. (1), (2) and (3) can all use a single Lua number to represent epoch time.

The complication here is (0), but this is hardly new to C APIs that have to compile across 32 and 64 bit base integer sizes. For example POSIX 93 added nanosleep which uses a struct timespec { time_t tv_sec; long tv_nsec; };. So one option would be for (1) builds to use a {tv_sec, tv_nsec} array. This would have the advantage that an epoche time was a single Lua variable on all build variants. The downside for (1) would be the storage and GC overheads of using an array. In terms of runtime performance it would be better to use a list pair, so an epoche time in (1) would be two arguments or return results, and my preference would be this option.

I have more to say, but I yield to other committers' comments first.

[jmattsson]

Option 2 is closest to our 5.1 when considering bit module usage.
I think sticking with option 1 as the initial drop is fine on the 8266 at least, and then as we cleanup 32/64 we can add configurability to enable 2 & 3. I do think there's value in both. On the esp32, as I've mentioned, option 1 might cause some grief unless the errata around hardware fp context switching has been resolved (or maybe it's okay because the LVM is tied to a single core).

For time handling, my vote would be return a sec,nsec pair.

I'd also really like to continue the work on merging the two branches so we only have to go through these things once, rather than twice :) Terry, when do you reckon that would be least disruptive? I would much prefer to not yank the rug from under you (again...) or any other serious contributor (though unless we clear the PR queue, there will of course be impacts to pending PRs as file will move around a lot with the switch to IDF folder structure).

[HHHartmann]

I'm also in favor of going on with option 1 for now.
I think it makes no sense to support too many variants. All of them have to be tested and we will have Lua 5.1 and Lua 5.3 already. And ESP32.
So having more than 2 variants for Lua 5.3 seems wrong to me.

For the time handling I would prefer one parameter instead of two. Just easier to pass around. But that is a weak preference, as I also see the benefits in the sec,nsec pair.

[umisef]

As a mere "user" with a whole bunch of existing devices out in the field (i.e. in a "if things break, the device we sold to the customer is a brick" situation, rather than "if it breaks, find the problem and reflash"), I have to say I would feel apprehensive about (1), for a couple of reasons:
a) time handling. Has been discussed a lot. The well-covered issue is handling sub-second resolution times. But even second-resolution uptimes exceed the resolution of a float after a little over 3 months; So if any uptime measurement accidentally is converted to a float in some code path, it WILL break eventually, but WON'T break quickly, so probably not during testing.
b) Similarly, with the 5.1 double-only builds, we would sometimes lose bits when a number turned into an integer (e.g. in the bit module), but resolution would never go below 32 bit., and in several places in our firmwares we rely on it. In scenario (1), I would be concerned about inadvertent conversions, and losing those bottom bits in existing code which was written assuming 32 bits minimum.

So we probably wouldn't be able to move existing firmwares over to 5.3 unless there was the option to build (2) or (3). And given that a lot of our code is on ESP32, where memory is less of an issue, it would be a shame to miss out on 5.3's improvements because of an effort to save memory.

Lastly, I am intrigued by the idea of (3). If each value is 12 bytes, anyway, using 64 bits for integers makes sense. And while doing 64 bit arithmetic on a 32 bit processor is slower than 32 bit, it's still a heck of a lot faster than floating point, and given the overhead of a byte code VM, I wonder how much of a performance difference there really would be....

[TerryE]

@umisef, yes it is fairly straightforward getting a clean compile for options (2) or (3) but regression testing and reengineering all of the modules so that they work as documented for all build variants would be a major exercise; who would resource this? It would be a lot less resource intensive to add a time resource type (which might be an 2 integer userdata under the hood).

Accidentally dropping into FP? The Lua rules for conversion to FP are well determined and you can always add a checkInteger() test to your code:

do
  local type, assert  = math.type, assert
  function checkInteger(i)
    assert(type(i) == 'integer')
  end
end

[pjsg]

@TerryE I think that you are making the same point as @umisef . The modules have not been well tested under the 32-bit float / 32-bit integer case (as indicated by the fact that there are tickets and pending PRs to fix the obvious breakages). Also, the '/' operator changes meaning -- in the 5.1 Integer build, this preserved precision, whereas in the 5.3 build, it converts to floating point, thereby losing precision.

The solution of preppering your code with checkInteger calls reveals how incompatible the two versions really are.

It appears that this change from 5.1 to 5.3 is a bigger change than moving from Python 2 to Python 3. And that took 10 years.

The approach of building in a native time datatype then leads you into a datatype that is as duration as well. Then you need to define operations on durations like multiply, divide etc.

[TerryE]

Python 2 to Python 3, PHP 5 to PHP 7, and Lua 5.1 to Lua 5.3 all face the same challenge: the language contains features that have proven over time to limit its ability to develop and to extend. (In Lua's case perhaps Lua 5.3 should have been better called Lua 6.0 because this is a pretty breaking change for production applications, IMO, and therefore not a minor level upgrade.) At some point the maintainers find it easier to give developers an option:

• move to the current version where feature enhancements can occur -- and accept the migration bump -- or
• stay with the old version and frozen support and with its limitations.

In PHP's case for example, over 50% of web services are still based on this and the majority of them still run a PHP 5 version. Lua is also used for high performance webservices, but in frameworks like Kobe + OpenResty + Nginx, that use the Lua 5.1 source compatible LuaJiT compiler.

I have already back-ported a number of API and performance enhancements into Lua 5.1 so that our library modules work with no (or occasionally minimal) LUA_VERSION conditional code. You correctly state that there are some "obvious breakages", and I agree: my stated goal was to get the our libraries + Lua RTS compiling clean under both versions, and to test core Lua and the core libraries (node, file, etc.) but to leave testing the other libraries to other committers and testers. Very little effort has been put into this, so it is hardly surprising that we have a few issues still to resolve.

Perhaps where we might disagree is that I class these as niggles (in that they are easily fixable with a small amount of effort in comparison to the total Lua53 project.)

Building a package to support exact time and intervals is straightforward thanks to the operator metamethods and a well-scoped small project. This could be done in Lua or C using the C API. Here is an example of a complex number package using this approach.

I have found this to be a very useful discussion, but it hasn't made me rethink our overall strategy. To be honest it is too late for that. What I do think this that we should expect to do a feature freeze on Lua 5.1 and offer some sort of Long Term Support for this version which include no enhancements and a very high threshold for bugfixes.

[pjsg]

Lua5.3 supports a number of combinations of integer and float (on big platforms). The core presumably supports all these options on ESP8266. For the lua5.3 version of the nodemcu-firmware, the single combination of 32-bit integer and 32-bit float was chosen. It was felt that the burden of supporting a new float type was worth the reduction in memory consumption. It is hoped that, fairly soon, all of nodemcu-firmware will support this combination.

I (and some other users) are trying to point out that this combination of 32-bit integer and 32-bit float does not solve their problems. I produced a very simple PR to support 32-bit integers and 64-bit floats. This was easy because all of the non-core lua stuff was originally coded to work with both 32-bit integers and 64-bit floats.

It was pointed out that a more standard arrangement would be 64-bit integers and 64-bit floats. I agreed, and updated the PR to allow selection of the 64-bit integer and 64-bit float combination as an option. This PR was bigger as I need to add -std=gnu11 to mbedtls. This then required add some more extern declarations to make the code compile cleanly. The core lua, of course, works fine. This is the version that I run at home and develop on. It appears to work fine (if I include my other PRs).

I think that I'm failing to understand the objections to my PR. Nobody is forced to run 64-bit integers and 64-bit doubles.

[TerryE]

I (and some other users) are trying to point out that this combination of 32-bit integer and 32-bit float does not solve their problems.

Philip, I do understand your point. My concern here isn't getting the Lua core to work with 64+64; it is the task of regressing and supporting this for all of our NodeMCU library modules, and that we will now need to test 3 live build configurations. IMO, this is a large task and a large incremental support burden.

Keeping Lua 5.1 is a perfectly good workaround for those users who need 64-bit FP working and "solves their problems" in the interim.

This development has been a big personal commitment for me -- the best part of 12 man-months of my effort to get so far; the end is in sight, and so I want to bring this milestone to a conclusion without further scope creep:

• a stable Lua 5.3 32 + 32 build including dual LFS and on-ESP LFS update
• an option to build with Lua 5.1 API alignment and current performance enhancements.

By all means continue experimenting with your own a 64-bit branch, and I am not suggest that you abandon this PR, but rather asking you put it on hold until the main Lua 5.3 32 + 32 work is stable and merged into dev.

To be honest if you or any other committers start complicating my work by merging this sort of PR into dev before we have got to this milestone, and thereby adding what I view as months more work for me to do at this stage, then I will probably abandon this project.

Once we have a stable Lua 5.3 32 + 32 build, if you then want to add support for 64 + 64, and you personally will commit to resource this work and the rework to ensure that our libraries support this 64+64 mode, then I will be happy to act as a reviewer. However, I believe that this would end up needing a few months solid development effort on your part.

[jmd13391]

I am a long time (0.9x days) NodeMCU User/Developer with thousands of NodeMCU based devices out in the world... which means I have practically zero input points here. However, I will input anyway...

I [@TerryE] will probably abandon this project

IMO that would be tragic. Yes, there have been valuable contributions from other committers but for me, it has been Terry's amazing labor of love work on the core and LFS that has made the real-world use of NodeMCU possible. Long ago, I consider him the "glue" that holds NodeMCU together. The politically correct, team player, let's all play together, all for one... one for all, group hug side of me says things should continue as they are; all committers have an equal say as to the direction this project takes and the weeks and weeks of debate/discussion on topics like this are beneficial. However, the "New Yorker" in me wants to shout-out: Hey, leave the *$!&! guy alone and let him finish his work.

There are times in all maturing projects like this when you need a tie-breaker vote. Folks, we need a "Chief Architect". If I had any say in it whatsoever, I would nominate @TerryE for the position immediately. IMO, he has without question, earned it.

For the [broken] record, I'll say it again... Terry should be allowed to get to the milestone he describes above (without interference or creep) and then the entire NodeMCU LUA51 platform should be frozen/abandoned in-place. Move-on with LUA53 and leave all the LUA51 baggage behind. I shudder to think of all the cool stuff all of the committers/contributors could have come up with by now had they not been engaged in seemingly endless discussions/debates about all the differences and challenges with trying to "common ground" NodeMCU across LUA51 and LUA53. [New Yorker] Your LUA51 wife has left you... get over it already. Stop crying in your breakfast cereal and go date that single LUA53 hottie that lives next door. 😎

[TerryE]

I should really apologise. I didn't mean for this to sound like an ultimatum. I guess that I was getting a bit stressed in the early hours of the morning. I just want to "lay this egg" without it growing further into an ostrich egg. I am sure that Philip won't mind holding off on his PR work for another month. I also accept that we will need to keep some level of support for Lua 5.1 for the foreseeable future, but I don't propose to do any more enhancements to this; just critical bugfixes.

[pjsg]

Terry I don't mind holding off until you are done. I think that the other PRs that fix some of the float/integer confusion in the C modules should go in fairly soon. Philip

…

On Fri, Jul 24, 2020 at 7:25 AM Terry Ellison ***@***.***> wrote: I should really apologise. I didn't mean for this to sound like an ultimatum. I guess that I was getting a bit stressed in the early hours of the morning. I just want to "lay this egg" without it growing further into an ostrich egg. I am sure that Philip won't mind holding off on his PR work for another month. I also accept that we will need to keep some level of support for Lua 5.1 for the foreseeable future, but I don't propose to do any more enhancements to this; just critical bugfixes. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#3224 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AALQLTOKFQUQNWFJMIMVDQTR5FVSBANCNFSM4PALXBIA> .