Giter Site home page Giter Site logo

Comments (15)

gvvaughan avatar gvvaughan commented on June 9, 2024

Hmmm... that doesn't work :-( I'll figure it out and make a new release to luarocks when I have it working.

Old versions of luaposix had all the functions at the top level, and I want to continue to (quietly) support that for backwards compatibility with code that was written with that assumption. The lazy loading is kind of a lie though, since requiring the top level posix module has to preload all of the submodules to make their functions available at the top level.

If you are writing new code, you should probably deliberately load only the submodules you actually need and call the thin wrapper functions from them. Unless, of course, you want to use the deprecated APIs and undocumented availability of submodule functions from the top-level posix module!

from luaposix.

parke avatar parke commented on June 9, 2024

For whatever it is worth:
I am writing new code.
I prefer the documentation to be accurate.
I prefer doing a single require 'posix', and accessing everything through the single master posix table via lazy loading.

Thank you for maintaining Luaposix, by the way.

Since you are considering publishing a new release of Luaposix, here is an idea:

Extend:
posix.unistd.write ( fd, buf )
To:
posix.unistd.write ( fd, buf, [offset, [count]] )

This would allow writing portions of buf without needing to create a substring for each portion. This is useful when performing non-blocking IO, for example.

If the above is a change that you would accept, I could possibly even write the pull request for you.

Also, exposing F_GETPIPE_SZ and F_SETPIPE_SZ on Linux might be nifty. (See man fcntl for details.)

from luaposix.

gvvaughan avatar gvvaughan commented on June 9, 2024

The submodules posix.unistd and the like are supposed to follow the C POSIX standard, so that you can mostly figure out how to use those calls from the C header files and/or manual pages, so I wouldn't want to change posix.unistd.write itself... but I would certainly accept a PR for an enhanced posix.write call in lib/posix/init.lua that accepts the additional optional arguments you propose.

Up until now, I've tried (and admittedly failed) to avoid adding Linux specific symbols to luaposix, mainly because I'd like for naive Lua code written against luaposix to automatically work on any platform where luaposix is available. However, if there's Linux specific code that you can't easily write without those symbols, the precedent has been set for them to be nil valued on hosts that don't support them... it does mean that any Lua code that uses them might need to check their values before using them and find a way to degrade gracefully rather than fall over (maybe by dropping some feature that relies on them). ...or we could export them without documenting it, and then I won't end up with a ton of support requests from people who can't run your code on macOS because luaposix is broken there ;-)

from luaposix.

parke avatar parke commented on June 9, 2024

The submodules posix.unistd and the like are supposed to follow the C POSIX standard, so that you can mostly figure out how to use those calls from the C header files and/or manual pages, so I wouldn't want to change posix.unistd.write itself... but I would certainly accept a PR for an enhanced posix.write call in lib/posix/init.lua that accepts the additional optional arguments you propose.

The functionality I am proposing is commonly done in C via pointer arithmetic, and therefore does not need to be part of the C/POSIX API. Lua, obviously, does not provide access to pointer arithmetic. Basically, I'm looking for a way to access such pointer arithmetic from Lua. If you would be interested in an enhanced C-implemented write() (perhaps called write_offset(), but there might be a better name) I would consider writing that. It would basically be a copy of write(), with the additional pointer-arithmetic functionality added.

However, if there's Linux specific code that you can't easily write without those symbols, [snip]

At present I have no pressing need for getting and setting buffer sizes.

from luaposix.

parke avatar parke commented on June 9, 2024

The submodules posix.unistd and the like are supposed to follow the C POSIX standard,

Another thought...

The current Luaposix write() API does already not follow the C POSIX standard, because it does not expose the count parameter.

ssize_t write(int fd, const void *buf, size_t count);

So my proposal, in a way, moves Luaposix closer to the POSIX API.

While I think the following is more intuitive...

posix.unistd.write ( fd, buf, [offset, [count]] )

... offset and count could be in the reverse order, as follows:

posix.unistd.write ( fd, buf, [count, [offset]] )

from luaposix.

gvvaughan avatar gvvaughan commented on June 9, 2024

Oh, good catch. Thanks. I wonder why nobody picked that up before?!?

Looking at the POSIX specification for write, we definitely need to add the nbyte parameter to write; and also a new binding for pwrite which matches your last proposal πŸ‘

If you can provide a PR that will speed things along, otherwise I'll be happy to get to it in the next week or two and then make a new release.

from luaposix.

parke avatar parke commented on June 9, 2024

Oh, good catch. Thanks. I wonder why nobody picked that up before?!?

Because whoever implemented the Luaposix write() API decided the C pair of (buf, nbyte) should be represented by a single Lua string.

However the C pair of (buf, nbyte) can also represent arbitrary substrings at near zero cost.

Whereas in Lua, all substrings always have a substantial cost in terms of CPU overhead and memory allocation.

Looking at the POSIX specification for write, we definitely need to add the nbyte parameter to write; and also a new binding for pwrite which matches your last proposal +1

Actually, I was unaware of pwrite(), and pwrite() does not match my last proposal.

My (now updated and revised) proposal is as follows. Extend Luaposix's write() and add pwrite() as follows:

function write  ( fd, buf, [nbyte, [buf_offset]] )
function pwrite ( fd, buf, nbyte, offset, [buf_offset] )

The above Lua functions would (after appropriate type and bounds checking) translate to the following C calls:

write  ( fd, buf + buf_offset, nbyte )
pwrite ( fd, buf + buf_offset, nbyte, offset )

The question is: How many substrings should we need to create to write a 16MB string to a non-blocking file descriptor?

Note that the creation of each substring may (will?) involve a hash table lookup, memory allocation, memcpy(), hash table insertion, and then eventual garbage collection!

All of these operations are completely avoidable with my proposed API.

With my proposal, we need to create (i.e. lookup, allocate, memcpy(), insert, and then garbage collect) exactly zero substrings. Whereas with the current Luaposix API, we may need to create 255 substrings! (Or maybe even many more than 255 substrings if the file descriptor buffer size is smaller than 64KB.)

And a naive implementation that creates those substrings with buf : sub ( buf_offset ) instead of buf : sub ( buf_offset, buf_offset + fd_buffer_size - 1 ) will end up creating (i.e. hash table lookup, allocate, memcpy(), hash table insertion, and garbage collection) a whopping two gigabytes of substring data to write a 16MB string to a non-blocking file descriptor. (And note that fd_buffer_size is unknown, and has to be deduced, which adds additional complexity to a non-naive implementation.)

An efficient implementation would need to copy only 16MB of data, but that is still 16MB of overhead that could be avoided altogether by exposing the above buf_offset functionality that is trivial in C but impossible in Lua.

If you can provide a PR that will speed things along, otherwise I'll be happy to get to it in the next week or two and then make a new release.

If you want my proposed buf_offset functionality, I will investigate creating a PR. Please let me know. Thanks!

from luaposix.

gvvaughan avatar gvvaughan commented on June 9, 2024

Thanks for the detailed write up. Your argument makes perfect sense to me!

I already added an optional nbytes parameter to the existing write binding earlier today. It seems like pwrite doesn't help your use case, so no reason to bind that before the next release (unless you are feeling super conscientious!)

I'd be delighted to accept a PR that implements the rest of your proposal, and make a fresh release right after merging it.

Thanks for the interesting discussion :-)

from luaposix.

parke avatar parke commented on June 9, 2024

It seems like pwrite doesn't help your use case, so no reason to bind that before the next release (unless you are feeling super conscientious!)

You are correct that I have no near term use for pwrite() (and maybe no long term use either, given that I was entirely unaware pwrite() even existed).

Fyi: Unsurprisingly, there is also a pread() function in POSIX.

I'd be delighted to accept a PR that implements the rest of your proposal, and make a fresh release right after merging it.

Okay, I'll try to create a PR.

Do you want buf_offset to be zero-based or one-based? I suggest zero-based. But I don't have a strong opinion. And I don't know what conventions Luaposix might already follow about exposing C-offsets to Lua programmers.

from luaposix.

parke avatar parke commented on June 9, 2024

I already added an optional nbytes parameter to the existing write binding earlier today.

Fyi: It appears your nbytes commit does not perform bounds checking.

from luaposix.

gvvaughan avatar gvvaughan commented on June 9, 2024

Fyi: Unsurprisingly, there is also a pread() function in POSIX.

That makes sense, but I'm in no hurry to add either until someone reports a need for the binding.

Do you want buf_offset to be zero-based or one-based? I suggest zero-based.

I agree. Lua has 1-based indexes, but this call specifically talks about offsets which are 0-based.

Fyi: It appears your nbytes commit does not perform bounds checking.

Hmm... that's a good point. The C function it binds does no bounds checking, so my knee jerk reaction was to follow suit with a minimal binding... but, you don't write in Lua for speed, so a bounds check is worth the tiny overhead.

If you don't mind adding nbytes bound checks in your PR, that would be great, otherwise I'll make sure to do so before release.

Thanks!

from luaposix.

parke avatar parke commented on June 9, 2024

Hmm... that's a good point. The C function it binds does no bounds checking, so my knee jerk reaction was to follow suit with a minimal binding... but, you don't write in Lua for speed, so a bounds check is worth the tiny overhead.

Aside: The C function cannot do bounds checking, as it lacks sufficient information to do so. (Such are the perils of C.)

If you don't mind adding nbytes bound checks in your PR, that would be great, otherwise I'll make sure to do so before release.

Okay, I'll add bounds checking.

from luaposix.

gvvaughan avatar gvvaughan commented on June 9, 2024

Ack.

I think the best error condition for out of bounds that follows existing conventions is to return nil, offset out of bounds, errno.ERANGE or similar. You might have to subvert pushresult to finagle that. errno.ERANGE might not be available on the host machine, but I think that's okay as long as the error message in return element two is descriptive.

All of that should be noted in the doc-comments too so that it is recorded into the generated ldocs for the release, please!

from luaposix.

gvvaughan avatar gvvaughan commented on June 9, 2024

Thought about this some more, and actually there's no need to co-opt ERANGE or whatever here, better to declare our own EOOB or whatever, make its value a negative integer so that it can never clash with anything exported by posix.errno, and then plumb it through strerror for completeness!

There's also an opportunity to return a much better error dynamic string along the lines of "offset 25 is out of bounds in 8 byte string 'foobar'" from a failed posix.unistd.write call because we're not constrained by the static errno table when performing our own inline boundary checks.

from luaposix.

parke avatar parke commented on June 9, 2024

I have replied at my pull request.

from luaposix.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    πŸ–– Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. πŸ“ŠπŸ“ˆπŸŽ‰

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❀️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.