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

[WIP] snapshot feature (issue 535) + many tiny changes #1196

Open
wants to merge 272 commits into
base: main
Choose a base branch
from

Conversation

MetrawareOfficial
Copy link

That's FYI.
That's WIP.
That's big.
That's my last day in the company, and the community can benefit.

  1. There are ~110 very clean commits of various improvements and fixes.
  2. There is Doxygen for its collaboration diagrams: see sObj and WrenCensus.
  3. There are ~160 not-so-perfect commits for the snapshot feature.

See dir example/embedding/snapshot
See file doc/notes/snapshot.md
See function wrenSnapshotWant() for usage.

Existing doc was (proof)read, but doc of snapshot feature is TODO.
Swizzle turns out nicely done in layers.
All existing tests but 5 pass: APITest_Run() tests more than stdout/stderr.
Test runner is improved.

Because stressing GC is useful:

The API is bad^Wlegacy.
For not learning premake, all code is badly bolted and spread.
The restored snapshot is not verified.

Sylvain HITIER added 30 commits March 13, 2025 10:45
…e field

Hence it's obvious that this generated bytecode is correct:
    CLASS                3 fields
Let's respect the separation of concerns.

BTW, fix reference: STORE_FIELD_THIS is faster than STORE_FIELD.

BTW, don't use backticks.
Say "short-circuit" evaluation.
Say "falsy" and "truthy".
But because they are included conditionally, include the configurator header
file before them.
A small negative value is more understandable than an enormous value.

Before:
    $ bin/wren_test_d test/language/deeply_nested_gc.wren
    -- gc --
    GC 104857717 before, 104774926 after (82791 collected), next at 157162389. Took 18.980ms.
    -- gc --
    GC 118516134 before, 118516342 after (18446744073709551408 collected), next at 177774513. Took 21.008ms.
    done

After:
    $ bin/wren_test_d test/language/deeply_nested_gc.wren
    -- gc --
    GC 104857717 before, 104774926 after (82791 collected), next at 157162389. Took 19.328ms.
    -- gc --
    GC 118516134 before, 118516342 after (-208 collected), next at 177774513. Took 21.123ms.
    done
- Add NOTE
- Fix doc
- Fix typo
- No need to `break` after `return`.
- Let's just `break` for the last `case`, and reach the only UNREACHABLE() left.
…AGGING

When WREN_NAN_TAGGING, numbers and objects are handled separately of its
own following `switch`.
But when not defined, handling them separately is redundant: let that be
handled by its own following `switch`.
…nd #endif

... for the (unique but important) non-trivial use of `#if WREN_NAN_TAGGING`.

It's long and made of only `#define`.
Thus the `#else` and `#endif` stand out.
Sylvain HITIER added 29 commits March 13, 2025 10:45
- This function was swrite().
- With commented-out handling of endianness.
- Use ctx->write.
... and the ByteBuffer it fills.

Keep slurpFile() for now, as it handles errors.
Mark it static.
... now that priv can be used instead.
Embed `read` and `write` members in an anonymous union.
Reorder `priv` just after this union.

=== Use case & rationale

1. A big machine writes a context.
2. The snapshot is transferred into a small embedded board.
3. At startup, the embedded board will read the context.
   But it will never write any. Thus, no need to store a unused NULL pointer.

The initializers are neatly approaching literate programming.
Although the `priv` name is typical in the Linux kernel [1], here it's a stream.

[1]: https://lwn.net/Articles/446317/
- WREN_SNAPSHOT:      what to do; each char selects an action
- WREN_SNAPSHOT_TO:   pathname into which save    a snapshot
- WREN_SNAPSHOT_FROM: pathname from which restore a snapshot
Saving verbosely is now independent from restoring verbosely.
... thus saved in the header.
The option and the shortened sizes are saved and restored.

But it's not yet effective on subsequent ids.
The subtle thing is the NULL pointer stored as a suitably long 0.

Subsume wrenFindInCtx().
Because ObjString are immutable, we can have several references to the same one
and save some memory.

Let's do that in:
- defineClass() when bootstrapping the core classes.
- compileInModule() when importing the core module.
... but one which works with wrenNewEmptyVM()
@mhermier
Copy link
Contributor

mhermier commented Mar 14, 2025 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants