GitLab

Using a function name that has kind info in it feels better in many
small ways (it's a nice sanity check; if we add coercion helper
methods for more kinds, it'll sure matter; etc).

And, that method on String types belongs on the type, not just on the
representation side.

And, drop the 'NewT' method for String types; instead, have them
generate use 'pkg.Style.T.FromString' pattern.

This seems to be getting pretty close: the only compile errors left
are about not finding "fromString" methods for key reification...
which is of course sensible, because that's not what those methods
have been called yet in the cases where they've shown up;
and they also haven't been placed on type-level styles yet.

So the next commit will be a little detour to touch over those methods
formerly known as "construct" and update them a bit to this pattern.

Fixes many issues with maybes.  Some minor typo-scale stuff; some
pretty consequential.  What to do about speciated lookup methods
still has big honking questions on it: returning a maybe there if
you didn't already have data in that format can be *not cheap*.
Figuring out the best DX there is gonna be interesting.

Making progress on how to handle keys, but slow.  Complex keys are...
well, really complex.  So far I've learned several things that *won't*
work, which is fun.  It's looking unlikely that having a KeyAssembler
type at all is actually going to be a good idea.

Complex keys are a behemoth of a feature, and it's gonna take
substantial effort to sort all of that out.

(Ain't that a lovely bunch of conditions?)

Previous comment in here missed an important case.

The reuse level is currently *very* low.

Some of that is intentional -- I'm writing radically non-DRY things
on purpose in many cases, because I want to see what *actually*
repeats most often before I start trying to remove repetitions.

Some of it is not intentional; it's because I just have no idea
how to organize units of reuse when doing templating.

This latter group could improve.

\o/

schema.StructField isn't hashable (in the golang sense of the word) --
*sometimes* -- at runtime -- because the Type field can contain things
that aren't hashable.  Such as schema.TypeStruct, since it contains
a slice of schema.StructField.  lol.

This was blocking the ability to use some types in other types.

If you've been following along for a while now, you don't need to see
the benchmarks to know what's coming.  The long story short is:
allocations are the root of all evil, and we got rid of some, and now
things are significantly faster.

Here's the numbers:

basicnode (just for a baseline to compare to):

```
BenchmarkMapStrInt_3n_AssembleStandard-8         1988986               588 ns/op             520 B/op          8 allocs/op
BenchmarkMapStrInt_3n_AssembleEntry-8            2158921               559 ns/op             520 B/op          8 allocs/op
BenchmarkMapStrInt_3n_Iteration-8               19679841                67.0 ns/op            16 B/op          1 allocs/op
BenchmarkSpec_Marshal_Map3StrInt-8               1377094               870 ns/op             544 B/op          7 allocs/op
BenchmarkSpec_Marshal_Map3StrInt_CodecNull-8     4560031               278 ns/op             176 B/op          3 allocs/op
BenchmarkSpec_Unmarshal_Map3StrInt-8              368763              3239 ns/op            1608 B/op         32 allocs/op
```

realgen, previously, using fcb:

```
BenchmarkMapStrInt_3n_AssembleStandard-8         4293072               278 ns/op             208 B/op          5 allocs/op
BenchmarkMapStrInt_3n_AssembleEntry-8            4643892               259 ns/op             208 B/op          5 allocs/op
BenchmarkMapStrInt_3n_Iteration-8               20307603                59.9 ns/op            16 B/op          1 allocs/op
BenchmarkSpec_Marshal_Map3StrInt-8               1346115               913 ns/op             544 B/op          7 allocs/op
BenchmarkSpec_Marshal_Map3StrInt_CodecNull-8     4606304               256 ns/op             176 B/op          3 allocs/op
BenchmarkSpec_Unmarshal_Map3StrInt-8              425662              2793 ns/op            1160 B/op         27 allocs/op
```

realgen, new, improved:

```
BenchmarkMapStrInt_3n_AssembleStandard-8         6138765               183 ns/op             129 B/op          3 allocs/op
BenchmarkMapStrInt_3n_AssembleEntry-8            7276795               176 ns/op             129 B/op          3 allocs/op
BenchmarkMapStrInt_3n_Iteration-8               19593212                67.2 ns/op            16 B/op          1 allocs/op
BenchmarkSpec_Marshal_Map3StrInt-8               1309916               912 ns/op             544 B/op          7 allocs/op
BenchmarkSpec_Marshal_Map3StrInt_CodecNull-8     4579935               257 ns/op             176 B/op          3 allocs/op
BenchmarkSpec_Unmarshal_Map3StrInt-8              465195              2599 ns/op            1080 B/op         25 allocs/op
```

So!  About 150% improvement on assembly between gen with fcb and our new-improved no-callback system.

And about 321% improvement in total now for codegen structs over the basicnode map.

That's the kind of ratio I was looking for :)

As with all of these measurements: these will also get much bigger on bigger corpuses.
Some of the improvements here are O(n) -> O(1), and some apply even more heartily in deeper trees, etc.
But it's telling that even on very small corpuses, the impact is already huge.

Results: mixed.

The good news:

The codegen works.

We were able to wire it to the standard benchmarks (!  great success).

It is flat out faster than any other implementation to date.

The not-so-good news:

It's not _as_ fast as I wanted >:(

The strategy of using a callback ("fcb") for transmitting 'finished'
signals from child assemblers to their parents causes an allocation.

That single source of allocations turns out to be one of the most
dominant things on the pprof of the benchmark.  (And it would
absolutely be even worse if 'N' was larger than '3' -- an alloc here
shifts us from an O(1) to O(n) on fields.)

So.  Good to know!  Having end to end benchmarks is VERY exciting.

And we're going to have to go back to the drawing board on that
part involving a callback.

This diff is pretty fun.  It's our first alternative representation,
and it all works out nicely (no internal syntactical complications
encountered or any other unexpected unpleasantness).

It's also our first representation that involves potentially recursive
destructuring work over a scalar!  So, you can see the new 'construct'
method conventions appearing to handle that.  Works out neatly.
This kind of recursive destructuring happens all within the span of
processing one "token" so to speak, so naturally it can work without
any stateful machinery.

Some utility functions are added to the mixins package.
(It's sort of betraying in the name of the package, but, well,
it's an extremely expedient choice.  See comments about import
management.  tl;dr: consistency makes things easier.)

Tests for a recursive case of this will be coming soon, but requires
addressing some other design flaws with the AdjunctConfig maps.
(StructField is... not a good key.  It's not always hashable... such
as when the StructField.Type field is inhabited by TypeStruct.)
Easy enough to fix, just not going to cram it into this commit.

The way null and the 'fcb' are handled here differs from previous
generators: here, we *always* have an 'fcb', and just make a special
one for the root builder.  This is... well, it works.  It was an
attempt to simplify things and generate fewer method overrides, and
it did.  However, I'm not going to dwell on this too long, because...

The 'fcb' system is not working out well overall for other reasons:
it's causing costly allocations.  (Taking a function pointer from
a method requires at least two words: one for the function pointer
and one for the value to apply it on.  That means an allocation.)
So a serious rewrite of anything involing the 'fcb' strategy is needed.

And that is going to be a little tricky.  The 'fcb' idea was itself
a trying-slightly-too-hard-to-be-clever attempt to avoid an alternative
solution that involves generating additional types per distinct child
value slot (and I'm still unwilling to pursue that one -- it suggests
far too many types).  The next best idea I have now seems to involve a
lot of pointers into other assembler's state machines; this will surely
be a bit touchy.  But no worse than any of the rest of all this, I
suppose: it's *all* "a bit touchy".  Buckle up for fun diffs ahead.)

So, considering these 'fcb' notes, this feels a bit mixed...
"Two steps forward, one step back", so to speak.  Still: progress!
And it's very exciting that we got through several new categories of
behavior without hitting any other new roadbumps.

The label used to advance over absent optionals is a compile-time error
if not referenced, so we have to branch generation for that.

I'll want this momentarily for doing Generate from other packages.
(I might not actually commit anything relating to this for a while,
but secretly, I'm playing with it.)

Correct a half-dozen laxnesses in path handling while at it.

Output is fairly fabulous.  Really happy with this.

Diff size is nice.  Took the opportunity to do some cleanups and the
net result is 193 insertions and 351 deletions counted by line.

Most importantly, though: running 'go test .' in the gengo package
actually generates, compiles, and tests **everything**.

The test file inside the "_test" dir that you previously had to jankily
run manually is gone.  Hooray!

And already, we've got three distinct packages being generated and
tested.  (Try 'diff _test/stroct*/tStroct.go'.  It's neat.)

The time overhead is acceptable so far.  The total time is up to 0.9sec
on my current machine.  Not as bad as feared.

Overall: nice.  I think this reduces the test friction to a point that
will significantly enable further progress.

Both commits on the merged branch contain long comments explaining why
that approach was not satisfying.

I'm feeling a bit salty about the whole thing, truth be told.
I really expected there to be APIs somewhere in this vicinity.

'go test' just really wasn't designed for this.  There's no first class
API-driven way to interact with it at all.  Even the 'go test -json'
capabilities, despite being built-in, appear to be strap-ons rather
than logically central consistent abstractions.

Where things can be parsed at all (and in general, they can't -- not
without significant potential for ambiguity slipping in from some
angle or another), trying to output them again in a normalized form
is seriously tricky business: a great deal of re-buffering is required.
(This diff doesn't have it: I started off far too optimistic about the
simplicity of the whole thing, and am putting it down rather than
continue.  I get the feeling I could tumble down the rabbit hole quite
some time and still only ever be *approaching* correctness
asymptotically -- never actually *getting* there.)

In addition, all the problems enumerated in the previous commit
('-v' needs to be passed down; '-run' is more or less impossible; and
all the sadness about closures and more helper files split across whole
package boundaries just for maximum distraction) are still here.

Yeah... let's... let's put this one back in the box.
I'm going to merge-ignore this and keep the plugin approach instead.
As much as I *really* don't want gcc as a test dep, all this stuff...
this seems worse.  (And maybe, someday, Go can be improved so plugins
don't have the gcc dep.  I'm going to hope for that.)

In the parent commit, we tried using plugins.  Turns out there's one
big, big drawback to plugins.  Building one invokes 'gcc'.
Works; as long as you... have gcc, and don't mind that having that as
a dependency that you now have to track and potentially worry about.
I don't consider that minor.

So, okay.  What's using 'go test' child invocations look like?

Well, bad.  It looks... bad.

The output is a mess.  Every child process starts its output as if its
a top level thing.  Why this happens is perfectly understandable and
hard to be mad at, but nonetheless the result is maddeningly illegible.
Cleaning this up might be possible, but will require some additional
effort to be invested.

There's some more irritating problems than that, though.
Notice the entirely new package that's sprouted.

Not being able to use closures in the test assertions is unfortunate.

Having to put the test assertions in *entirely different files* than
their setup... now that's really painful.

(And yeah, sure, we can move the setup out to another package, too.
It's not pleasing, though.  Then what of the test is actually in the
package it's testing?  The entrypoint?  So then, I'd end up editting
the test setup and assertions in one package (but running tests on
that package would actually no-op; syke!) and then have to switch to
the other package to actually run it?  There's no angle this can be
sliced that results in any sort of _nice_.)

Flags like '-v' aren't inherited now, either.  Hooray.
We could try to hack that in, I suppose.  Detect '-test.v' arg.
But it's just another thing that needs patching up to be shipshape.

Speaking of which, '-run' arg isn't even *remotely* manageable
without massive effort.  The thing can contain regexps, for goodness
sake -- how can we possibly semantically modify those for passdown?

I'm not sure where to go with this.  Depending on gcc in tests is
undesirable.  But all these other effects from trying to use 'go test'
this way seem even *more* undesirable.

Using golang's plugin feature, we can... well, *do* this.

To date, testing codegen has involved running the "test" in the gen
package to get it to emit code; and then switching to the emitted
package and _manually_ running the tests there.

Now, running `go test` in the gen package is sufficient to do
*everything*: both the generation, and the result compiling,
and we can even write tests against the interfaces and run those,
all in one step.

There's also lots of stuff that becomes possible now that we can easily
generate multiple separate packages with various codegen outputs:

- Overall: everything is granular.  We can test selections of things,
  rather than needing to have everything fall into place at once.
- Generally more organized results.
- We can more easily inspect the size of generated code.
- We can more easily inspect the size of the compiled result of gen!
  (Okay, not really.  I'm seeing a '.so' file that's 4MB is coming out
  from 200sloc of "String".  I don't think that's exactly informative.
  Some constant factor is thoroughly obscuring the data of interest.
  Nice idea in theory though, and maybe we'll get there later.)
- We can diff the generated type for variations in adjunct config!
  (Probably not something that will end up tested, but neat to be able
  to do during dev.)

Doing this with go plugins seemed like the neatest way to do this.
It's certainly not the only way, though.  And in particular, I will
confess that this will probably make developing from a windows host
pretty painful: go plugins aren't supported on windows.  Mind,
this doesn't mean you can't *use* codegen or its results on windows.
It just means the tests won't work.  So, someone doing development
_on the codegen itself_ would have to wait for the CI server to run
the tests on their behalf.  Hopefully this is survivable.

(There's also a fun wee wiggle in that loading a plugin has the
requirement that it be built with the same "runtime".  The definition
of "runtime" here happens to include whether or not things have been
built in "race" mode.  So, the '-race' flag disappears from our CI
config file in this diff; otherwise, CI will get the (rather confusing)
error "plugin was built with a different version of package runtime".
This isn't really worrying to ditch, though.  I'm... not even sure why
the '-race' was in our CI script in the first place.  Must've arrived
via cargo cult; we don't _have_ any concurrency in this library.)

An alternative way to go about all this would be to have the tests for
gen invoke `go test` (rather than `go build` in plugin mode) on each of
the generated packages.  It strikes me as similar but worse.
We still have to invoke the go tools from inside the test;
we'd have *more* work to do to either copy tests into the gen'd package
or else generate calls back to the parent package for test functions
(which still have to be written against interfaces, so that they can
compile even when the gen isn't done, as it indeed isn't when you
freshly check out the repo -- exact same as with the plugin approach);
and in exchange for the extra work, we get markedly worse output
('go test' doesn't nest nicely, afaict), and we can't separate the
compiling of the generated code from the evaluation of tests on it,
and we'd have no possibility of passing information via closures should
we wish to develop table-driven tests where this would be useful.
tl;dr: Highest cost, uglier, and worse.

No matter which way we go about this, there *is* a speed trade-off.
Invoking the compiler per package adds at least a half second of time
for each package, in practice.  Worth it, though.

And on the off chance that this plugin approach does burn later,
and we do want to switch to child process 'go test' invocations...
the good news is: we shouldn't actually have to rewrite very much.
The everything-starts-from-NodeStyle-and-tests-against-Node work is
exactly the same for making the plugin approach work, and will
trivially pivot to working fine in for child 'go test' approaches,
should we find it necessary to do so in the future.  So!  With our
butts covered: a plugin'ing we shall go!

Some of the code here still needs cleanup; this is a proof of concept
checkpointing commit.  (The real thing probably won't have such
function names as "TestFancier".)  But, we do get to see here:
plugins work; more than one in the process works; and they work even
when the same type names are in the generated packages.  All good.

Some touches to ImplicitValue came along for the ride, but aren't
exercised yet.

Tests are getting unwieldy.  I think something must be done about
this before proceding much further or it's going to start resulting
in increasingly significant velocity loss.

Still have a lot of uncertainty to resolve around nullable and optional
in general.  But writing that up elsewhere -- trying to treat it from
the spec angle without getting the implementation too mixed up in it.

I was particularly worried about trailing optionals.  Happily,
they appear to work fine.

Representation iterators and smooth stopping included.

Starting to wonder how best to organize these tests to continue.
333 lines already?  Questionable scalability.

Seems to be on track after all.

I started doing double-duty in tests to keep the volume down:
the block for nullables tests both 'nullable' and 'nullable optional';
and the optionals similarly.  Will make failures require more careful
reading to discern, but probably a fine trade.

I'm almost a little staggered.  We've got structures with maybes
both using pointers and not, and everything seems fine.

A few fixes were necessary, but they were mostly "d'oh" grade.
(The finishCallback logic is a bit hairy, but appears correct now.)

More cases for more variations in presence coming up next.

Sanity check level: gen result compiles.

Next: time to target tests at all this jazz that actually exercise
and run the generated code.

Easier than previous commit message seemed to think.

Since we already have finishCallback functions in play in any scene
where maybes can also be in play, we can just make those responsible
for copying out a pointer from the child assembler to the parent value.
That makes the child assembler able to create a new value late in
its lifecycle and still expect it can land in a proper home.

Tests needed badly; this is *oodles* of edge cases.
Unrelated fix needed first.  (Might make that elsewhere and rebase
this to include that, to reduce the carnage ever so slightly.)

Fix many issues with maybes; adjunct config for if maybe is implemented using a pointer; attempt the finishCallback system for reusable child assemblers.

This... almost appears to be on a dang nice track.

Except one fairly critical thing.  It doesn't work correctly if your
maybe IS implemented as containing a pointer.

Because that pointer starts life as nil in the parent structure.

And that's hard to fix.

We can't have a pointer to a pointer in the assembler;
that'd cause the type bifructation we've been trying to avoid.
(We can give up and do that of course; it would be correct.
Just... more code and larger final assembly size.
And if we did this, a lot of this diff would be rewritten.)

We could have the parent structure allocate a blank of the field,
and put a pointer to it in the maybe and also in the child assembler.
Except... this is a wasted allocation if it turns out to be null.

Rock and hard place detected in a paired configuration.
Might have to back out of this approach entirely.  Not sure.

We'll see how this works out.

Lots of tasty bitshuffling in this commit.

Remaining: creating, embedding, and returning child value assemblers.
This might involve a full type per field: doing so would be direct,
have pretty excellent verbosity in debugging symbols,
and pointers back to parent suffer no interface indirection;
overall, it's probably going to have the fastest results.
However, it would also mean: we can't save resident memory size if we
have more than one field in a struct that has the same type;
and we we can't avoid increasing binary size if the same types are used
as fields in several places.
Going to pause to think about this for a while.