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

Rewrite pretty printer #477

Closed
wants to merge 1 commit into from
Closed

Rewrite pretty printer #477

wants to merge 1 commit into from

Conversation

Kmeakin
Copy link
Contributor

@Kmeakin Kmeakin commented Jan 26, 2023

Rewrote the surface pretty printer using the algorithm from A New Language-Independent Prettyprinting Algorithm. The resulting code is a little imperative compared to the old functional style, but I think it should be possible to write a functional wrapper around the imperative algorithm.

TODO:

  • Tweak heuristics to match the old pretty printing output
  • Avoid emitting trailing whitespace at the end of lines

@Kmeakin
Copy link
Contributor Author

Kmeakin commented Jan 26, 2023

Benchmark results

  • Peak heap usage, as measured by heaptrack ./target/release/fathom data --module formats/opentype.fathom /usr/share/fonts/liberation/LiberationMono-Bold.ttf
    • main: 142.9MB
    • new-pretty: 10.4MB
  • Runtime, as measured by hyperfine -w 1 -r 10 "./target/release/fathom data --module formats/opentype.fathom /usr/share/fonts/liberation/LiberationMono-Bold.ttf":
    • main: 328.9 ms ± 4.0 ms
    • new-pretty: 24.0 ms ± 0.5 ms

@Kmeakin Kmeakin force-pushed the new-pretty branch 4 times, most recently from 2ce338d to 21cccbf Compare January 27, 2023 11:13
@Kmeakin
Copy link
Contributor Author

Kmeakin commented Jan 28, 2023

Abandoned in favour of #483

@Kmeakin Kmeakin closed this Jan 28, 2023
@Kmeakin Kmeakin deleted the new-pretty branch January 30, 2023 12:07
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