The new writer has reached parity of the old, with the exception of some
edge case explicit error handling that should never occur (which will be
added), and cleanup/docs.
Removing this flag now allows me to perform that cleanup without having to
worry about updating the now-old implementation.
I ran `tameld` with the new writer against our production system with
numerous programs and a significant number of test cases, and diff'd the old
and new xmle files, and everything looks good.
This is a significant milestone, in the sense that it is the culmination of
the past month or so of work to prove that an Iterator-based XIR will be
viable for the system.
This barely had any impact on the performance from the previous commit
reporting the profiling. This performs at least as well as the quick-xml
based writer. In isolated benchmarks, it performs better, but in the real
world, the linker spends most of its time reading xmlo files, and so minor
differences in writing do not have a significant overall impact.
With that said, a lot of cleanup and documentation is still needed. That is
the subject of the upcoming commits, before this writer can finalized.
The previous iterators had to be used in a certain order because they mixed
concerns, out of concern for performance. This attempts to chain even more
iterators to see how it may perform.
To be clear: this will be cleaned up. This was just an experiment.
Here were profiles on the average of 50 runs of linking our largest program:
Baseline, pre-XIR (with fragments removed from output) 0.8082
XIR writer, pre-ElemWrap, no #[inline] 0.7844s
XIR writer, ElemWrap, no #[inline] 0.7918s
XIR writer, ElemWrap, inlines in obj::xmle::xir 0.7892s
XIR writer, ElemWrap, inlines in obj::xmle::xir and ir::asg::section 0.7858s
XIR writer, ElemWrap, inline in only ir::asg::section 0.781s
Pre-ElemWrap, inlines in ir::asg::section 0.7772s
These profiles are difficult, because they hit the filesystem so much. I
write to /dev/null, but it reads 100s of xmlo files from disk.
It's clear that the impact is fairly modest and within a margin of error; as
such, I will continue down the path of writing code that's easier to grok
and maintain, since not doing so would be a micro-optimization relative to
the concerns of the rest of the system at this point.
But the purpose of all of this work was to determine whether an
iterator-based XIR would be viable. It seems to be competitive. I'll
finish up the writer reimplementation and move on.
Two reasons for this:
1. It's unnecessary, since it's the same ref, so long as we actually build
everything as part of the stage job; and
2. In our environment, the token used doesn't have access to pull from the
registry.
Fixing the latter item can be done at another time.
This contains some awkward coupling for opening and closing tags to reduce
the complexity of the `Iterator` types that must be manually
specified. That may be addressed shortly.
This was creating a heap-allocated `Vec` for each map symbol despite not
actually needing it. We do have multiple froms for return map values.
But by the time we may want this type of thing, we'll have a different IR
for it anyway.
See the docs for a much deeper discussion. In summary: traits do not
support static methods, and this is the workaround, which relies on unstable
nightly constant function features.
This implementation is tested using `qname_const!`, and will be utilized
with a new static type in a following commit.
This is to support two things:
1. Early switch to 2021 Edition, which is stable Oct 21; and
2. To make use of unstable const features.
The rationale is that switching to nightly does not really have any
significant downside for us, given that TAMER is used only by us and
the only risk is that unstable features may change a bit, which can be
mitigated with certain precautions.
The rationale for each unstable feature will be documented as they are used,
including documentation on what would be required to remove it and what
functionality would be lost / need to change in doing so.
This is far from fully documented; it's just a start. I'll document fully
once the implementation is done, to ensure I don't waste time documenting
things that may change.
These are getting large and messy.
And I now notice that I never completed the header test after
prototyping. Shame on me.
Also, errata from the previous commit message: the diffs are identical
_except for attribute escaping_ that is unnecessary; we're outputting data
read directly from existing XML files (output by Saxon), so characters are
already escaped as needed.
DEV-10561
The `l:dep` section of the `xmle` file, after formatting (since XIR writes
without newlines and indentation), is now identical to the existing xmle
writer. I can now move on to the other sections.
Note that the attribute movement in this commit is simply to get the diff to
properly align. Once the current xmle writer is removed, I'll organize them
a bit more sensibly.
`obj::xmle::xir` also needs documentation, now that it's shown to be viable.
The new xmle writer was having to intern before write, which did not make
sense.
This continues with consistently using symbols throughout the system, and
is a smaller size than `String` as a bonus.
`IdentKind` needs to be written to `xmle` files and displayed in error
messages. String slices were used when quick-xml was used for writing,
which will be going away with the new writer.
This has been a long time coming, and has been repeatedly stashed as other
parts of the system have evolved to support it. The introduction of the XIR
tree was to write tests for this (which are sloppy atm).
This currently writes out the `xmle` header and _most_ of the `l:dep`
section; it's missing the object-type-specific attributes. There is,
relatively speaking, not much more work to do here.
The feature flag `wip-xir-xmle-writer` was introduced to toggle this system
in place of `XmleWriter`. Initial benchmarks show that it will be
competitive with the quick-xml-based writer, but remember that is not the
goal: the purpose of this is to test XIR in a production system before we
continue to implement it for a frontend, and to refactor so that we do not
have multiple implementations writing XML files (once we echo the source XML
files).
I'm excited to get this done with so that I can move on. This has been
rather exhausting.
The 16-bit interner at present will be used only for span contexts. In the
future, this interner may become specialized specifically for that, but for
now let's just re-use what we already have so that I can move on.
DEV-10733
I want to make it clear in the assertion that the problem could be caused by
duplicate strings. We do not sort by string, because in part we may in the
future want to group certain symbols together in some arbitrary way so we
can compare ranges (using the markers).
If that doesn't end up happening, it may be better to just sort by string
to obviate the problem.
It's really awkward not having them caps, when not only are constants
expected to be, but also that we cannot maintain consistency between the
string and the identifier name in even the simplest of cases.
(We could use `r#`, but that's too cumbersome.)
`StaticSymbolId` was created before the more specific types, which render it
unnecessary. If we need a generic type, it can be re-introduced, but using
`static_symbol_newtypes!`.
This is the interner that is intended to be used with the majority of the
system; the 16-bit interner is left around for the moment, but will likely
later become specialized.
This had the writing on the wall all the same as the `'i` interner lifetime
that came before it. It was too much of a maintenance burden trying to
accommodate both 16-bit and 32-bit symbols generically.
There is a situation where we do still want 16-bit symbols---the
`Span`. Therefore, I have left generic support for symbol sizes, as well as
the different global interners, but `SymbolId` now defaults to 32-bit, as
does `Asg`. Further, the size parameter has been removed from the rest of
the code, with the exception of `Span`.
This cleans things up quite a bit, and is much nicer to work with. If we
want 16-bit symbols in the future for packing to increase CPU cache
performance, we can handle that situation then in that specific case; it's a
premature optimization that's not at all worth the effort here.
We'll see how the syntax evolves over time. It's not ideal to have to
specify the type, rather than having the compiler infer it, but I don't much
feel like getting into my first procedural macro right now, so we'll stick
with this approach for the time being.
This will set the stage to be able to safely e.g. create QNames statically
at compile-time and would allow us to make any attempts to bypass it
unsafe.
Previously, we were allocating only u32 versions of `SymbolId` for the
statically allocated symbols. This introduces a new symbol type with a very
small datatype (8 bits) that is able to cast into any `SymbolId`. This is
explained in the docs.
We'll be taking this typing further in future commits so that static symbols
are better-suited for compile-time guarantees for static newtype
construction.
DEV-10710
This is the beginning of static symbols, which is becoming increasing
necessary as it's quite a pain to have to deal with interning static strings
any place they're used.
It's _more_ of a pain to do that in conjunction with newtypes (e.g. `QName`,
`AttValue`, etc) that make use of `SymbolId`; this will allow us to
construct _those_ statically as well, and additional work to support that
will be coming up.
DEV-10701
These were using GiB of memory, which is ...unnecessary.
I reduced the iteration count significantly, but it was still wasting a lot
of time and memory and needed `with_capacity` to reduce the number of copies
after reallocation.
It is not typical that a buffer would contain this much information.
This broke when I removed `SelfClose`. I used to run
`make all fmt check bench` before every push, but they take a while to run,
in part because it uses nightly and has to recompile too.
But it looks like I need to be more diligent again.
This is exactly was I said I was _not_ going to do in the previous commit,
but apparently hacking late at night had me forget the whole reason that
XIRT is being introduced now---unit tests. I'll be emitting a XIR stream
and I need to parse it for convenience in the tests.
So, here's a good start. Next will be some generalizations that are useful
for the tests as well. This is pretty bare, but accomplishes the task.
See docs for more info.
The `tree` module is getting more difficult to navigate. The tests still
remain where they were, since a bunch of concerns are mixed together. Any
tests specific only to this module will be added here.
This is implemented only for the writer, since its use case is to be able to
concatenate strings without copying during writing.
It doesn't really make sense to support this in XIR Tree, since a reader
should never produce this. But if we ever run into this (e.g. due to some
internal processing pipeline), we'll address it then; XIR Tree might have to
do copying, then, but should probably wait until encountering all fragments
before interning. That'd be a distraction right now.
This commit will make more sense once the broader context is committed, but
it's needed for lowering from `Sections` into a XIR stream.
This will also change once we pre-allocate symbols, like rustc, when the
interner is initialized.
This is my first use of the `paste` crate, which is used to generate
identifiers. So this is partly an experiment, and it seems much better than
having to write a proc macro, at least at this point in time. If this code
stays around, it'll probably be generalized further and used elsewhere, but
I'd prefer not to go this route long-term.
This moves some logic into `ElementStack` (which would be part of `Stack` if
variants were their own types), rather than peering so deeply into its
data.
This correctly retains and restores the parent stack after processing an
attribute for a child element.
This does increase the size of [`Stack`] a bit, but we can evaluate whether
it's too large at a later time. It's currently 832 bits with `Ix=u32`,
which is large, but the question is whether it matters; we'll see as we
begin to use it.
This moves most of the parsing logic into `Stack`, which rightfully owns the
stack manipulation and state transitions. `ParserState` becomes exactly
what it says it is---a management of the persistent state of the parser, and
is also responsible for digesting tokens and dispatching their data to the
proper event.
This approach has a number of benefits over the old design: it's
self-documenting, making the intent clear; and it is easier to reason about
the subset of states (for both humans and Rusts) than a large match of
transitions.
This contains a number of TODO items that will be addressed shortly. It
also obviated that the previous commit was incomplete---it doesn't persist
`pstack` for attributes on child elements! That'll be fixed too.
Mike Gerwitz
Austin Schaffer