Skip to content
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

Addition of file.savedump(filename, func [, striplevel]) #3145

Open
TerryE opened this issue Jun 5, 2020 · 6 comments
Open

Addition of file.savedump(filename, func [, striplevel]) #3145

TerryE opened this issue Jun 5, 2020 · 6 comments
Assignees

Comments

@TerryE
Copy link
Collaborator

TerryE commented Jun 5, 2020

Missing feature

file.putcontents(filename, string.dump(func)) is a simple method of copying a given in-RAM or in-LFS function to an LC file in SPIFFS, however the string.dump() C code does

  luaL_Buffer b;
  luaL_buffinit(L,&b);
  lua_dump(L, writer, &b);
  luaL_pushresult(&b);

and at peak this generates three copies of the function in memory:

  • the executable Proto hierarchy
  • the dump in the luaL_Buffer chunks on the Lua stack
  • their concatenation into a single TString on the stack.

This is approach is fine PC class environments but we only have ~40Kb RAM and this limits the size of function we can dump. If we use a writer that sends directly to file, then we don't need the extra RAM to store the 2 copies of the full dump.

The main use of this function is to allow developers to dump LFS functions to LC files for #2917.

Note:

  • Because this is a node function, this would be available in both the Lua 5.1 and 5.3 environments.
  • It makes sense to add the striplevel parameter to node.compile() as well.

Justification

To remove an unnecessary RAM constraint is dumping in-RAM or in-LFS functions.

Workarounds

file.putcontents(filename, string.dump(func)) only works for function with dumps less than 15Kb or so. No workarounds for functions larger than this.

@TerryE TerryE self-assigned this Jun 5, 2020
@nwf
Copy link
Member

nwf commented Jun 6, 2020

Because this is a node function, this would be available in both the Lua 5.1 and 5.3 environments.

Is file not available in both 5.1 and 5.3??

I think I would advocate for this to be file.dump() rather than node.dumpfile(), just so all SPIFFS/Fat functionality remains grouped together. In terms of technical content, tho', I approve entirely.

@TerryE
Copy link
Collaborator Author

TerryE commented Jun 6, 2020

OK, your suggestion of putting this function in file works well, but given that we can only have one top level function per undump, I think that this better a file function rather than a file.obj method -- that is the filename should be parameter, and file.xxxxfile() doesn't sound right to me so why not:

  file.savedump(filename, func [, striplevel])

@nwf
Copy link
Member

nwf commented Jun 6, 2020

I wonder... given that you have made a lovely pipe module for us... could we have a more generic "dump to pipe" function and pull from the pipe to the filesystem (or even elsewhere, e.g., the network) as appropriate? Could we undump from a pipe similarly? We'd need some back-pressure mechanism on pipe if it's not already so capable?

@TerryE
Copy link
Collaborator Author

TerryE commented Jun 7, 2020

I wonder... given that you have made a lovely pipe module for us...

This is more a case of using the right tool for the job. We needed the pipe module because the net interface is non-blocking (very much the same as I/O within node.js is) and we need to marshal packets for network efficiency. File I/O is synchronous, and in this case Lua already provides the lua_Writer for such synchronous uses, so it makes sense to use this.

@TerryE TerryE changed the title Addition of node.dumpfile(filename, func [, striplevel]) Addition of file.savedump(filename, func [, striplevel]) Jun 7, 2020
@stale
Copy link

stale bot commented Jun 2, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Jun 2, 2021
@stale stale bot closed this as completed Jun 20, 2021
@TerryE
Copy link
Collaborator Author

TerryE commented Sep 26, 2021

Don't close

@TerryE TerryE reopened this Sep 26, 2021
@stale stale bot removed the stale label Sep 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants