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

rework colors and display code #7

Merged
merged 24 commits into from
Jun 24, 2024
Merged

rework colors and display code #7

merged 24 commits into from
Jun 24, 2024

Conversation

iximeow
Copy link
Owner

@iximeow iximeow commented Jun 24, 2024

the changelog says it better than another repetition:

## 0.3.0

added a new crate feature flag, `alloc`.
  this flag is for any features that do not require std, but do require
  containers from `liballoc`. good examples are `alloc::string::String` or
  `alloc::vec::Vec`.

added `yaxpeax_arch::display::DisplaySink` after revisiting output colorization.
  `DisplaySink` is better suited for general markup, rather than being focused
  specifically on ANSI/console text coloring. `YaxColors` also simply does not
  style text in some unfortunate circumstances, such as when the console that
  needs to be styled is only written to after intermediate buffering.

  `DisplaySink` also includes specializable functions for writing text to an
  output, and the implementation for `alloc::string::String` takes advantage of
  this: writing through `impl DisplaySink for String` will often be substantially
  more performant than writing through `fmt::Write`.

added `mod color_new`:
  this includes an alternate vision for `YaxColors` and better fits with the
  new `DisplaySink` machinery; ANSI-style text markup can be done through the
  new `yaxpeax_arch::color_new::ansi::AnsiDisplaySink`.

  this provides more flexibility than i'd initially expected! yours truly will
  be using this to render instructions with HTML spans (rather than ANSI
  sequences) to colorize dis.yaxpeax.net.

  in the future, `mod colored` will be removed, `mod color_new` will be renamed
  to `mod color`.

deprecated `mod colored`:
  generally, colorization of text is a presentation issue; `trait Colorize`
  mixed formatting of data to text with how that text is presented, but that is
  at odds with the same text being presented in different ways for which
  colorization is not generic. for example, rendering an instruction as marked
  up HTML involves coloring in an entirely different way than rendering an
  instruction with ANSI sequences for a VT100-like terminal.

added `yaxpeax_arch::safer_unchecked` to aid in testing use of unchecked methods
  these were originally added to improve yaxpeax-x86 testing:
  https://github.com/iximeow/yaxpeax-x86/pull/17, but are being pulled into
  yaxpeax-arch as they're generally applicable and overall wonderful tools.
  thank you again 522!

added `mod testkit`:
  this module contains tools to validate the correctness of crates implementing
  `yaxpeax-arch` traits. these initial tools are focused on validating the
  correctness of functions that write to `DisplaySink`, especially that span
  management is correct.

  `yaxpeax-x86`, for example, will imminently have fuzz targets to use these
  types for its own validation.

made VecSink's `records` private. instead of extracting records from the struct
  by accessing this field directly, call `VecSink::into_inner()`.

made VecSink is now available through the `alloc` feature flag as well as `std`.

meta: the major omission in this release is an architecture-agnostic way to
format an instruction into a `DisplaySink`. i haven't been able to figure out
quite the right shape for that! it is fully expected in the future, and will
probably end up somehow referenced through `yaxpeax_arch::Arch`.

(includes more than just colors and display adjustments because the new code also uses things like safer_unchecked that used to live in yaxpeax-x86 and really ought to be shared more broadly)

this... in a very roundabout way, starts to help with #5, as well

iximeow added 24 commits June 22, 2024 11:03
it was built in-place around yaxpeax-x86, hoisted out once it seemed
suitable and could be generalized. yay!

also include a Makefile in yaxpeax-arch now to test that various crate
feature flag combinations.. work.
... this makes the doc test not exist under nostd, yes. `VecSink` does
not have a no-std alternative that makes the example reasonable.
@iximeow iximeow merged commit 053b8c8 into no-gods-no- Jun 24, 2024
1 check passed
iximeow added a commit to iximeow/yaxpeax-x86 that referenced this pull request Jun 24, 2024
this empty commit reproduces a github comment that describes the work on
commits from this point back to, roughly, 1.2.2. since many commits
between these two points are interesting in the context of performance
optimization (especially uarch-relevant tweaks), many WIP commits are
preserved. as a result there is no clear squash merge, and this commit
will be the next best thing.

on Rust 1.68.0 and a Xeon E3-1230 V2, relative changes are measured
roughly as:
  starting at ed4f238:
    - non-fmt ns/decode: 15ns
    - non-fmt instructions/decode: 94.6
    - non-fmt IPC: 1.71
    - fmt ns/decode+display: 91ns
    - fmt instructions/decode+display: 683.8
    - fmt IPC: 2.035

  ending at 6a5ea10
    - non-fmt ns/decode: 15ns
    - non-fmt instructions/decode: 94.6
    - non-fmt IPC: 1.71
    - fmt ns/decode+display: 47ns
    - fmt instructions/decode+display: 329.6
    - fmt IPC: 1.898

for an overall ~50% reduction in runtimes to display instructions.
writing into InstructionTextBuffer reduces overhead another ~10%.

-- original message follows --

this is where much of iximeow/yaxpeax-arch#7
originated.

`std::fmt` as a primary writing mechanism has.. some limitations:
* rust-lang/rust#92993 (comment)
* llvm/llvm-project#87440
* rust-lang/rust#122770

and some more interesting more fundamental limitations - writing to a
`T: fmt::Write` means implementations don't know if it's possible to
write bytes in reverse order (useful for printing digits) or if it's OK
to write too many bytes and then only advance `len` by the correct
amount (useful for copying variable-length-but-short strings like
register names). these are both perfectly fine to a `String` or `Vec`,
less fine to do to a file descriptor like stdout.

at the same time, `Colorize` and traits depending on it are very broken,
for reasons described in yaxpeax-arch.

so, this adapts `yaxpeax-x86` to use the new `DisplaySink` type for
writing, with optimizations where appropriate and output spans for
certain kinds of tokens - registers, integers, opcodes, etc. it's not
a perfect replacement for Colorize-to-ANSI-supporting-outputs but it's
more flexible and i think can be made right.

along the way this completes the move of `safer_unchecked` out to
yaxpeax-arch (ty @5225225 it's still so useful), cleans up some docs,
and comes with a few new test cases.

because of the major version bump of yaxpeax-arch, and because this
removes most functionality of the Colorize impl - it prints the
correct words, just without coloring - this is itself a major version
bump to 2.0.0. yay! this in turn is a good point to change the
`Opcode` enums from being tuple-like to struct-like, and i've done so
in
1b8019d.

full notes in CHANGELOG ofc. this is notes for myself when i'm trying
to remember any of this in two years :)
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.

1 participant