Page MenuHomeFreeBSD

stand: lua: enhance lfs.dir() to speed up kernels_autodetect
ClosedPublic

Authored by kevans on Dec 10 2020, 4:01 PM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Nov 8, 11:04 PM
Unknown Object (File)
Oct 9 2024, 7:51 AM
Unknown Object (File)
Oct 5 2024, 6:03 AM
Unknown Object (File)
Oct 2 2024, 9:11 AM
Unknown Object (File)
Sep 25 2024, 3:40 AM
Unknown Object (File)
Sep 24 2024, 8:41 PM
Unknown Object (File)
Sep 24 2024, 8:41 PM
Unknown Object (File)
Sep 24 2024, 8:41 PM
Subscribers
None

Details

Summary

This eliminates a lot of stat() calls that happen when lualoader renders the menu with the default settings, and greatly speeds up rendering on my laptop.

ftype is nil if loader/loader.efi hasn't been updated yet, falling back to lfs.attributes() to test.

This is technically incompatible with lfs, but not in a particularly terrible way.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Is kernel.KERNCONF actually a convention? I tend to use INSTKERNNAME and 'test.KERNCONF'.

I like the existing behavior better but don't object to changing the default if it's noticeably slower on some systems. I would really prefer we pass through d_type and check DT_DIR as a first step (see line comments), but I won't insist on it.

stand/lua/core.lua
243–255

This is an unfortunate shortcoming of the lfs API we copied from Lua. Our direntries have d_type == DT_DIR and we shouldn't need the extra stat to determine directory or not. One way to speed this up without loss of functionality would be to pass that information through to Lua.

In D27542#615616, @cem wrote:

Is kernel.KERNCONF actually a convention? I tend to use INSTKERNNAME and 'test.KERNCONF'.

Well, I thought it was. :-) It turns out this is just the convention for pkgbase, but I've not done non-pkgbase installs of multiple kernels in years.

stand/lua/core.lua
243–255

Good point. I think this would be easy enough to bolt on to lfs in a way that isn't egregiously incompatible. We could switch this to using the iteration protocol manually (it's only a little uglier) and add a method to the iterator's metatable to grab it.

kevans retitled this revision from stand: add new default kernels_autodetect="strict" to stand: lua: enhance lfs.dir() to speed up kernels_autodetect.
kevans edited the summary of this revision. (Show Details)

Make kernels_autodetect more efficient instead. This is slightly different than the approach I had previously described, because I remembered how these things are structured... just return the type as a second return value for inspection.

I like the approach, thanks!

libexec/flua/modules/lfs.c
126

const struct dirent *entry

129

Maybe add a comment about why we're only exposing this non-standard lfs extension to the loader.

134–137

If we're doing strings, it might make sense to just match the lfs.attributes(,"mode") strings — "directory", etc.

We could also expose some integer constants in lfs namespace and match on those? If strings are more canonical in Lua it's fine to stay with strings.

libexec/flua/modules/lfs.c
134–137

Now that you mention it, we might as well just export lfs.DT_DIR for loader. I'll drop DT_REG in the process, because I still haven't come up with a situation where I care for it.

kevans marked 4 inline comments as done.

Export lfs.DT_DIR, comment about the incompatibility, constify

This revision is now accepted and ready to land.Jan 26 2021, 3:08 PM