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

Improve clearing Fennels macro-loaded table when needed. #117

Open
rktjmp opened this issue Sep 3, 2023 · 2 comments
Open

Improve clearing Fennels macro-loaded table when needed. #117

rktjmp opened this issue Sep 3, 2023 · 2 comments
Labels

Comments

@rktjmp
Copy link
Owner

rktjmp commented Sep 3, 2023

Lua is retaining macros modules in memory, so edits to macro files are not reflected in auto-builds without restarting neovim.

Fennel retains a package.loaded style list of macro modules it has seen, which we need to clear, exists as macro-loaded in specials.fnl, but appears to be exposed on the compiler env at some points.

@rktjmp rktjmp added the external label Sep 4, 2023
@rktjmp
Copy link
Owner Author

rktjmp commented Sep 4, 2023

    (each [k _ (pairs fennel.macro-loaded)]
      (tset fennel.macro-loaded k nil))

in compiler.compile-string fixes issue, but means every compile call will fetch and compile macro modules.

  • Will give slower compile performance:
    • make (manually) -- probably not a huge deal, the act of typing is probably slower than the build,
    • auto-make (on save) -- noticeable,
    • diagnostics (during editing) -- will extend delay when leaving text buffer ,
    • require (when compile needed).
  • Will give better UX when:
    • Editing files that have had macros change (eg: adding x to m1 wont show "m1 does not have x function" when importing it without restart.
    • auto-make (actually functional), this is probably the bigger issue as it can "say" building was all ok but the macros have not changed, so the code has not changed.

In some respects I think the performance isn't a huge deal. Compiling is already "slow" compared to anything else, so it being a bit slower (probably not to a noticeable degree unless the given files use lots of complex macros) might be a reasonable trade off for the better diagnostics and clearer-functioning auto-make.

Potentially the compile could be run in its own thread, which gets its own state. Either a fresh thread could be built per-compile-entry (probably not "per compile call"), which would incur some start up time and "config" time where hotpot is loaded again, etc (this is ~7-8ms), or a persistent thread could exist that compiles when needed, and nukes the macros-loaded table each time but the performance hit wont be as "felt".

The off-thread compile has been considered before (#92, was blocked waiting on nvim 0.8) when working on ruin.fnl which has very complex macros, and is particularly slow when editing large test files that generate 10x lua code.

Next:

  • See viability of only nuking when make is running. Passing an extra flag to the compiler? The check requires fennel in memory, but that could be checked "in make"? If fennel exists, nuke the table, otherwise there is no need, fennel will load in the compiler fresh anyway. Least impact on performance, adds another "quirk" to that branch.
  • Otherwise include the above each loop.

Then,

  • See viability of off-thread compilation.
    • adds dragons, might still be worth it.

rktjmp added a commit that referenced this issue Sep 4, 2023
Macro modules are retained by fennel in a macros-loaded table. If we
compile using a macro, then edit the macro, then recompile, we would end
up using the older in-memory macro and generate incorrect code.

By clearing the macros-loaded table before calling compile, we can
enforce that the latest macro code is loaded.

Currently this fix only applies to make, but could be extended to
diagnostics.

See #117.
@rktjmp rktjmp changed the title Macros retained in memory Fennels macro-loaded table causes stale macro codegen Sep 4, 2023
@rktjmp
Copy link
Owner Author

rktjmp commented Sep 4, 2023

Mitigation in 9297ab3

@rktjmp rktjmp changed the title Fennels macro-loaded table causes stale macro codegen Improve clearing Fennels macro-loaded table when needed. Sep 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant