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.

Sign up for GitHub

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

Jump to bottom

add conditional formatting for Terminal #651

Merged
merged 3 commits into from
Mar 7, 2024
Merged

add conditional formatting for Terminal #651

merged 3 commits into from
Mar 7, 2024

Conversation

RCasatta
Copy link
Contributor

@RCasatta RCasatta commented Mar 5, 2024

fmt functions produce big binaries.

cargo bloat --release --example big --all-features -n 1000000 --full-fn | grep astelem | grep fmt 0.0% 0.1% 2.8KiB miniscript miniscript::miniscript::astelem::>::fmt::h75ba3ce6da61d374 0.0% 0.1% 2.8KiB miniscript miniscript::miniscript::astelem::>::fmt::h6786aa26184b9ff6 0.0% 0.1% 2.8KiB miniscript miniscript::miniscript::astelem::>::fmt::h03f9c2155559a440 0.0% 0.1% 2.8KiB miniscript miniscript::miniscript::astelem::>::fmt::h00e63af9b88389b4 0.0% 0.1% 2.8KiB miniscript miniscript::miniscript::astelem::>::fmt::hb60190c99fd48d17 0.0% 0.1% 2.8KiB miniscript miniscript::miniscript::astelem::>::fmt::ha9001c527b4cc4d6 0.0% 0.1% 2.8KiB miniscript miniscript::miniscript::astelem::>::fmt::ha726829fe4f3bf2b 0.0% 0.1% 2.8KiB miniscript miniscript::miniscript::astelem::>::fmt::h95a7eadaa261d645 0.0% 0.1% 2.8KiB miniscript miniscript::miniscript::astelem::>::fmt::h819e09dcb99bbc52 0.0% 0.1% 2.8KiB miniscript miniscript::miniscript::astelem::>::fmt::h6ef89d30f9f4cb57 0.0% 0.1% 2.8KiB miniscript miniscript::miniscript::astelem::>::fmt::h693edb5e35f9fc08 0.0% 0.1% 2.8KiB miniscript miniscript::miniscript::astelem::>::fmt::h27891ccf9609c782 0.0% 0.1% 2.7KiB miniscript miniscript::miniscript::astelem::>::fmt::he0a63893fe76f527 0.0% 0.1% 2.7KiB miniscript miniscript::miniscript::astelem::>::fmt::h86eba44cb7298c4f 0.0% 0.1% 2.7KiB miniscript miniscript::miniscript::astelem::>::fmt::h37a4628264df8c3b 0.0% 0.1% 2.7KiB miniscript miniscript::miniscript::astelem::>::fmt::h2f3a9d297ec35dea 0.0% 0.1% 2.7KiB miniscript miniscript::miniscript::astelem::>::fmt::h879ad74922a70f16 0.0% 0.1% 2.7KiB miniscript miniscript::miniscript::astelem::>::fmt::h7dc2d9bc27c88fab 0.0% 0.1% 2.7KiB miniscript miniscript::miniscript::astelem::>::fmt::h7578352fd420fc88 0.0% 0.1% 2.7KiB miniscript miniscript::miniscript::astelem::>::fmt::h1809b9931776159e 0.0% 0.1% 2.7KiB miniscript miniscript::miniscript::astelem::>::fmt::hf7760e0f27d70db4 0.0% 0.1% 2.7KiB miniscript miniscript::miniscript::astelem::>::fmt::h9adfe0d7e2bfdd42 0.0% 0.1% 2.7KiB miniscript miniscript::miniscript::astelem::>::fmt::h61cdb06e2a061932 0.0% 0.1% 2.7KiB miniscript miniscript::miniscript::astelem::>::fmt::h44e1ab5daacde0df 0.0% 0.1% 2.7KiB miniscript miniscript::miniscript::astelem::>::fmt::h2646ddd57694241b 0.0% 0.1% 2.7KiB miniscript miniscript::miniscript::astelem::>::fmt::he31fd2e9b12960f9 0.0% 0.1% 2.7KiB miniscript miniscript::miniscript::astelem::>::fmt::hbefb77c4050bd5c9 0.0% 0.1% 2.7KiB miniscript miniscript::miniscript::astelem::>::fmt::hb4deb7ef951af7af 0.0% 0.1% 2.7KiB miniscript miniscript::miniscript::astelem::>::fmt::h80ee1108b955f2b4 0.0% 0.1% 2.6KiB miniscript miniscript::miniscript::astelem::>::fmt::hc779f37cb81f761a 0.0% 0.1% 2.6KiB miniscript miniscript::miniscript::astelem::>::fmt::hc4ac4a9b598fbdfb 0.0% 0.1% 2.6KiB miniscript miniscript::miniscript::astelem::>::fmt::h5cca31d73f30c3c7 0.0% 0.1% 2.6KiB miniscript miniscript::miniscript::astelem::>::fmt::h4588410e445d16c6 0.0% 0.1% 2.6KiB miniscript miniscript::miniscript::astelem::>::fmt::heb60f683c9e55e36 0.0% 0.1% 2.6KiB miniscript miniscript::miniscript::astelem::>::fmt::hbe7939a797fcce35 0.0% 0.1% 2.6KiB miniscript miniscript::miniscript::astelem::>::fmt::ha536af3ff4313ed1 0.0% 0.1% 2.6KiB miniscript miniscript::miniscript::astelem::>::fmt::h8969ddab726dea7f 0.0% 0.1% 2.6KiB miniscript miniscript::miniscript::astelem::>::fmt::h7fca0d80a428dd24 0.0% 0.1% 2.6KiB miniscript miniscript::miniscript::astelem::>::fmt::h6b4909db760a4721 0.0% 0.1% 2.6KiB miniscript miniscript::miniscript::astelem::>::fmt::h5ed643a399966d49 0.0% 0.1% 2.6KiB miniscript miniscript::miniscript::astelem::>::fmt::h1ed7d512c859a5a9 0.0% 0.1% 2.6KiB miniscript miniscript::miniscript::astelem::>::fmt::hfd9775e798913648 0.0% 0.1% 2.6KiB miniscript miniscript::miniscript::astelem::>::fmt::hf87563509ea7253d 0.0% 0.1% 2.6KiB miniscript miniscript::miniscript::astelem::>::fmt::hb8b75c9c988d95c3 0.0% 0.1% 2.6KiB miniscript miniscript::miniscript::astelem::>::fmt::h87b650968eb119ba 0.0% 0.1% 2.6KiB miniscript miniscript::miniscript::astelem::>::fmt::h9986ae4314d85479 0.0% 0.1% 2.6KiB miniscript miniscript::miniscript::astelem::>::fmt::h74343d637646e38a 0.0% 0.1% 2.6KiB miniscript miniscript::miniscript::astelem::>::fmt::h680da8fa0ec0142c 0.0% 0.1% 2.6KiB miniscript miniscript::miniscript::astelem::>::fmt::h6197658ba03f1fdf 0.0% 0.1% 2.6KiB miniscript miniscript::miniscript::astelem::>::fmt::h961e1a4b75732a11 0.0% 0.1% 2.6KiB miniscript miniscript::miniscript::astelem::>::fmt::h57b3cd148371177a 0.0% 0.1% 2.6KiB miniscript miniscript::miniscript::astelem::>::fmt::h13d43f019a96d314 0.0% 0.1% 2.6KiB miniscript miniscript::miniscript::astelem::>::fmt::h008ca416c1ebc0d7 0.0% 0.1% 2.5KiB miniscript miniscript::miniscript::astelem::>::fmt::h82a27926db80dc37 0.0% 0.1% 2.5KiB miniscript miniscript::miniscript::astelem::>::fmt::hc12b3098c3979175 0.0% 0.1% 2.5KiB miniscript miniscript::miniscript::astelem::>::fmt::h76e624d7abc545ba 0.0% 0.1% 2.5KiB miniscript miniscript::miniscript::astelem::>::fmt::h24511846d004e114 0.0% 0.1% 2.5KiB miniscript miniscript::miniscript::astelem::>::fmt::h0d96e9d943f04d7d

Since Debug and Display implementations are ~equal except the Debug or Display representation of its child, this introduce conditional fmt trading performance for code size. Since performance shouldn't be that important in string conversion seems an interesting trade-off.

This is a PoC since the final impl of conditional_fmt is a little tedious, would like a concept ack before proceding

@apoelstra
Copy link
Member

Since performance shouldn't be that important in string conversion seems an interesting trade-off.

Performance is absolutely critical here because fmt::Display winds up being called multiple times within single log lines.

@apoelstra
Copy link
Member

Having said that, adding a single branch (which should be predicted correctly ~100% of the time) and one function call per fmt is not much of a perf hit.

So, concept ACK reducing performance in this way. But I'm a little torn about how much uglier/harder to maintain this makes the code.

@RCasatta
Copy link
Contributor Author

RCasatta commented Mar 5, 2024 •
edited
Loading

But I'm a little torn about how much uglier/harder to maintain this makes the code.

It is uglier, but in a way is more maintainable, since there aren't 2 different places to update.

@RCasatta
Copy link
Contributor Author

RCasatta commented Mar 5, 2024

Let's implement and see how much it impacts the code size

@apoelstra
Copy link
Member

Sounds good. FYI at least one of the CI failures is real -- you need to run the formatter.

@RCasatta
Copy link
Contributor Author

RCasatta commented Mar 5, 2024 •
edited
Loading

I used the total given by this call to benchmark
cargo bloat --release --example big --all-features -n 1000000 --full-fn --no-relative-size --filter miniscript::miniscript::astelem

master | 354.4KiB
1st commit | 311.2KiB
2nd commit | 304.1KiB

Note that much of the total filtered space is from unrelated fn (from_tree and encode) because I can't filter only for touched functions, so percentual savings are greater.

Note that I tryed to be equivalent to master but I don't know why there is difference in Debug and Display in where there is the call to wrap_char in Debug is done "before" and in Display "after" can we make it uniform or is it on purpose?

cargo bloat after 2nd commit 
    Size      Crate Name
 10.3KiB miniscript miniscript::miniscript::astelem::<impl miniscript::expression::FromTree for miniscript::miniscript::decode::Terminal<Pk,Ctx>>::from_tree::h9f0e21a6984f4330
 10.3KiB miniscript miniscript::miniscript::astelem::<impl miniscript::expression::FromTree for miniscript::miniscript::decode::Terminal<Pk,Ctx>>::from_tree::h7ad801121c70ebf8
  9.8KiB miniscript miniscript::miniscript::astelem::<impl miniscript::expression::FromTree for miniscript::miniscript::decode::Terminal<Pk,Ctx>>::from_tree::h8afc1f5741cc6c88
  9.8KiB miniscript miniscript::miniscript::astelem::<impl miniscript::expression::FromTree for miniscript::miniscript::decode::Terminal<Pk,Ctx>>::from_tree::hfbb3eea546f0ed5e
  9.8KiB miniscript miniscript::miniscript::astelem::<impl miniscript::expression::FromTree for miniscript::miniscript::decode::Terminal<Pk,Ctx>>::from_tree::h83f11da0940e39f4
  9.8KiB miniscript miniscript::miniscript::astelem::<impl miniscript::expression::FromTree for miniscript::miniscript::decode::Terminal<Pk,Ctx>>::from_tree::hf69e9d38ebcf6f12
  9.8KiB miniscript miniscript::miniscript::astelem::<impl miniscript::expression::FromTree for miniscript::miniscript::decode::Terminal<Pk,Ctx>>::from_tree::h3488d5210911d228
  9.6KiB miniscript miniscript::miniscript::astelem::<impl miniscript::expression::FromTree for miniscript::miniscript::decode::Terminal<Pk,Ctx>>::from_tree::h7b6c5674ba1e6c1c
  9.6KiB miniscript miniscript::miniscript::astelem::<impl miniscript::expression::FromTree for miniscript::miniscript::decode::Terminal<Pk,Ctx>>::from_tree::hde372a9d4b5c5e18
  9.6KiB miniscript miniscript::miniscript::astelem::<impl miniscript::expression::FromTree for miniscript::miniscript::decode::Terminal<Pk,Ctx>>::from_tree::hce662838637e682d
  9.5KiB miniscript miniscript::miniscript::astelem::<impl miniscript::expression::FromTree for miniscript::miniscript::decode::Terminal<Pk,Ctx>>::from_tree::hb9348ab42b4e068b
  9.5KiB miniscript miniscript::miniscript::astelem::<impl miniscript::expression::FromTree for miniscript::miniscript::decode::Terminal<Pk,Ctx>>::from_tree::h27e4aa6d40370098
  9.2KiB miniscript miniscript::miniscript::astelem::<impl miniscript::expression::FromTree for miniscript::miniscript::decode::Terminal<Pk,Ctx>>::from_tree::hdd80f9ed1a505c79
  9.2KiB miniscript miniscript::miniscript::astelem::<impl miniscript::expression::FromTree for miniscript::miniscript::decode::Terminal<Pk,Ctx>>::from_tree::hc05415dcda43ea37
  9.2KiB miniscript miniscript::miniscript::astelem::<impl miniscript::expression::FromTree for miniscript::miniscript::decode::Terminal<Pk,Ctx>>::from_tree::h5acde483a00f3278
  9.2KiB miniscript miniscript::miniscript::astelem::<impl miniscript::expression::FromTree for miniscript::miniscript::decode::Terminal<Pk,Ctx>>::from_tree::h02e6cb64d69d80ec
  4.4KiB miniscript miniscript::miniscript::astelem::<impl miniscript::miniscript::decode::Terminal<Pk,Ctx>>::encode::h8c1c2c97697638d0
  4.4KiB miniscript miniscript::miniscript::astelem::<impl miniscript::miniscript::decode::Terminal<Pk,Ctx>>::encode::hf5613b992604354f
  4.0KiB miniscript miniscript::miniscript::astelem::<impl miniscript::miniscript::decode::Terminal<Pk,Ctx>>::encode::h7602b18de1d79170
  4.0KiB miniscript miniscript::miniscript::astelem::<impl miniscript::miniscript::decode::Terminal<Pk,Ctx>>::encode::h085189c0b9436f4b
  3.9KiB miniscript miniscript::miniscript::astelem::<impl miniscript::miniscript::decode::Terminal<Pk,Ctx>>::encode::heae74b35fe563802
  3.9KiB miniscript miniscript::miniscript::astelem::<impl miniscript::miniscript::decode::Terminal<Pk,Ctx>>::encode::hb121508e8a05ce55
  3.9KiB miniscript miniscript::miniscript::astelem::<impl miniscript::miniscript::decode::Terminal<Pk,Ctx>>::encode::he02b60cc8636eb76
  3.9KiB miniscript miniscript::miniscript::astelem::<impl miniscript::miniscript::decode::Terminal<Pk,Ctx>>::encode::h19d0d8805c870a16
  3.7KiB miniscript miniscript::miniscript::astelem::<impl miniscript::miniscript::decode::Terminal<Pk,Ctx>>::encode::h6c3f9514f063ebca
  3.6KiB miniscript miniscript::miniscript::astelem::<impl miniscript::miniscript::decode::Terminal<Pk,Ctx>>::encode::h7584ddf7e4d86ca4
  1.9KiB miniscript miniscript::miniscript::astelem::<impl miniscript::miniscript::decode::Terminal<Pk,Ctx>>::conditional_fmt::hac0280e50a6bff0e
  1.9KiB miniscript miniscript::miniscript::astelem::<impl miniscript::miniscript::decode::Terminal<Pk,Ctx>>::conditional_fmt::h9f2950db41d5613b
  1.9KiB miniscript miniscript::miniscript::astelem::<impl miniscript::miniscript::decode::Terminal<Pk,Ctx>>::conditional_fmt::h5d7c92315f1c6cf9
  1.9KiB miniscript miniscript::miniscript::astelem::<impl miniscript::miniscript::decode::Terminal<Pk,Ctx>>::conditional_fmt::hc4b11777107ae610
  1.9KiB miniscript miniscript::miniscript::astelem::<impl miniscript::miniscript::decode::Terminal<Pk,Ctx>>::conditional_fmt::h9d5fd5ca433278a9
  1.9KiB miniscript miniscript::miniscript::astelem::<impl miniscript::miniscript::decode::Terminal<Pk,Ctx>>::conditional_fmt::h5ba65b6cec16f7c1
  1.9KiB miniscript miniscript::miniscript::astelem::<impl miniscript::miniscript::decode::Terminal<Pk,Ctx>>::conditional_fmt::h4d13b1a531542921
  1.9KiB miniscript miniscript::miniscript::astelem::<impl miniscript::miniscript::decode::Terminal<Pk,Ctx>>::conditional_fmt::hdeae28696610f07d
  1.9KiB miniscript miniscript::miniscript::astelem::<impl miniscript::miniscript::decode::Terminal<Pk,Ctx>>::conditional_fmt::h8f48674eb899e11c
  1.9KiB miniscript miniscript::miniscript::astelem::<impl miniscript::miniscript::decode::Terminal<Pk,Ctx>>::conditional_fmt::h3c8e70afe704764d
  1.9KiB miniscript miniscript::miniscript::astelem::<impl miniscript::miniscript::decode::Terminal<Pk,Ctx>>::conditional_fmt::h324447961c8c1212
  1.9KiB miniscript miniscript::miniscript::astelem::<impl miniscript::miniscript::decode::Terminal<Pk,Ctx>>::conditional_fmt::hdb717bc9f51b89c1
  1.9KiB miniscript miniscript::miniscript::astelem::<impl miniscript::miniscript::decode::Terminal<Pk,Ctx>>::conditional_fmt::h5ba411b9fd6440bf
  1.9KiB miniscript miniscript::miniscript::astelem::<impl miniscript::miniscript::decode::Terminal<Pk,Ctx>>::conditional_fmt::h25c555cb36c25b05
  1.9KiB miniscript miniscript::miniscript::astelem::<impl miniscript::miniscript::decode::Terminal<Pk,Ctx>>::conditional_fmt::h14352795dccd3738
  1.9KiB miniscript miniscript::miniscript::astelem::<impl miniscript::miniscript::decode::Terminal<Pk,Ctx>>::conditional_fmt::hf14db66e578be9e0
  1.9KiB miniscript miniscript::miniscript::astelem::<impl miniscript::miniscript::decode::Terminal<Pk,Ctx>>::conditional_fmt::hb51d956b8679520a
  1.9KiB miniscript miniscript::miniscript::astelem::<impl miniscript::miniscript::decode::Terminal<Pk,Ctx>>::conditional_fmt::h77da983dd7d98fe0
  1.9KiB miniscript miniscript::miniscript::astelem::<impl miniscript::miniscript::decode::Terminal<Pk,Ctx>>::conditional_fmt::h6c87931664abcd9c
  1.9KiB miniscript miniscript::miniscript::astelem::<impl miniscript::miniscript::decode::Terminal<Pk,Ctx>>::conditional_fmt::h52144da7296a5e6e
  1.8KiB miniscript miniscript::miniscript::astelem::<impl miniscript::miniscript::decode::Terminal<Pk,Ctx>>::conditional_fmt::hf99ceabc6a3492c8
  1.8KiB miniscript miniscript::miniscript::astelem::<impl miniscript::miniscript::decode::Terminal<Pk,Ctx>>::conditional_fmt::he40dd69ef602964c
  1.8KiB miniscript miniscript::miniscript::astelem::<impl miniscript::miniscript::decode::Terminal<Pk,Ctx>>::conditional_fmt::ha97624eabe263664
  1.8KiB miniscript miniscript::miniscript::astelem::<impl miniscript::miniscript::decode::Terminal<Pk,Ctx>>::conditional_fmt::h294f1513fa9c5e10
  1.8KiB miniscript miniscript::miniscript::astelem::<impl miniscript::miniscript::decode::Terminal<Pk,Ctx>>::conditional_fmt::hb10f2a8c68b747ad
  1.8KiB miniscript miniscript::miniscript::astelem::<impl miniscript::miniscript::decode::Terminal<Pk,Ctx>>::conditional_fmt::he5753ad3a2aa9df8
  1.8KiB miniscript miniscript::miniscript::astelem::<impl miniscript::miniscript::decode::Terminal<Pk,Ctx>>::conditional_fmt::h0b489a0ae3fe48e0
  1.8KiB miniscript miniscript::miniscript::astelem::<impl miniscript::miniscript::decode::Terminal<Pk,Ctx>>::conditional_fmt::h09265b4fe75686b1
  1.8KiB miniscript miniscript::miniscript::astelem::<impl miniscript::miniscript::decode::Terminal<Pk,Ctx>>::conditional_fmt::h027ec1a6867a373e
    821B miniscript miniscript::miniscript::astelem::<impl core::fmt::Debug for miniscript::miniscript::decode::Terminal<Pk,Ctx>>::fmt::he0a63893fe76f527
    821B miniscript miniscript::miniscript::astelem::<impl core::fmt::Debug for miniscript::miniscript::decode::Terminal<Pk,Ctx>>::fmt::h86eba44cb7298c4f
    821B miniscript miniscript::miniscript::astelem::<impl core::fmt::Debug for miniscript::miniscript::decode::Terminal<Pk,Ctx>>::fmt::h75ba3ce6da61d374
    821B miniscript miniscript::miniscript::astelem::<impl core::fmt::Debug for miniscript::miniscript::decode::Terminal<Pk,Ctx>>::fmt::h6786aa26184b9ff6
    821B miniscript miniscript::miniscript::astelem::<impl core::fmt::Debug for miniscript::miniscript::decode::Terminal<Pk,Ctx>>::fmt::h37a4628264df8c3b
    821B miniscript miniscript::miniscript::astelem::<impl core::fmt::Debug for miniscript::miniscript::decode::Terminal<Pk,Ctx>>::fmt::h2f3a9d297ec35dea
    821B miniscript miniscript::miniscript::astelem::<impl core::fmt::Debug for miniscript::miniscript::decode::Terminal<Pk,Ctx>>::fmt::h2646ddd57694241b
    821B miniscript miniscript::miniscript::astelem::<impl core::fmt::Debug for miniscript::miniscript::decode::Terminal<Pk,Ctx>>::fmt::h03f9c2155559a440
    821B miniscript miniscript::miniscript::astelem::<impl core::fmt::Debug for miniscript::miniscript::decode::Terminal<Pk,Ctx>>::fmt::h00e63af9b88389b4
    821B miniscript miniscript::miniscript::astelem::<impl core::fmt::Debug for miniscript::miniscript::decode::Terminal<Pk,Ctx>>::fmt::hf7760e0f27d70db4
    821B miniscript miniscript::miniscript::astelem::<impl core::fmt::Debug for miniscript::miniscript::decode::Terminal<Pk,Ctx>>::fmt::he31fd2e9b12960f9
    821B miniscript miniscript::miniscript::astelem::<impl core::fmt::Debug for miniscript::miniscript::decode::Terminal<Pk,Ctx>>::fmt::hbefb77c4050bd5c9
    821B miniscript miniscript::miniscript::astelem::<impl core::fmt::Debug for miniscript::miniscript::decode::Terminal<Pk,Ctx>>::fmt::hb60190c99fd48d17
    821B miniscript miniscript::miniscript::astelem::<impl core::fmt::Debug for miniscript::miniscript::decode::Terminal<Pk,Ctx>>::fmt::hb4deb7ef951af7af
    821B miniscript miniscript::miniscript::astelem::<impl core::fmt::Debug for miniscript::miniscript::decode::Terminal<Pk,Ctx>>::fmt::ha9001c527b4cc4d6
    821B miniscript miniscript::miniscript::astelem::<impl core::fmt::Debug for miniscript::miniscript::decode::Terminal<Pk,Ctx>>::fmt::ha726829fe4f3bf2b
    821B miniscript miniscript::miniscript::astelem::<impl core::fmt::Debug for miniscript::miniscript::decode::Terminal<Pk,Ctx>>::fmt::h9adfe0d7e2bfdd42
    821B miniscript miniscript::miniscript::astelem::<impl core::fmt::Debug for miniscript::miniscript::decode::Terminal<Pk,Ctx>>::fmt::h95a7eadaa261d645
    821B miniscript miniscript::miniscript::astelem::<impl core::fmt::Debug for miniscript::miniscript::decode::Terminal<Pk,Ctx>>::fmt::h879ad74922a70f16
    821B miniscript miniscript::miniscript::astelem::<impl core::fmt::Debug for miniscript::miniscript::decode::Terminal<Pk,Ctx>>::fmt::h819e09dcb99bbc52
    821B miniscript miniscript::miniscript::astelem::<impl core::fmt::Debug for miniscript::miniscript::decode::Terminal<Pk,Ctx>>::fmt::h80ee1108b955f2b4
    821B miniscript miniscript::miniscript::astelem::<impl core::fmt::Debug for miniscript::miniscript::decode::Terminal<Pk,Ctx>>::fmt::h7dc2d9bc27c88fab
    821B miniscript miniscript::miniscript::astelem::<impl core::fmt::Debug for miniscript::miniscript::decode::Terminal<Pk,Ctx>>::fmt::h7578352fd420fc88
    821B miniscript miniscript::miniscript::astelem::<impl core::fmt::Debug for miniscript::miniscript::decode::Terminal<Pk,Ctx>>::fmt::h6ef89d30f9f4cb57
    821B miniscript miniscript::miniscript::astelem::<impl core::fmt::Debug for miniscript::miniscript::decode::Terminal<Pk,Ctx>>::fmt::h693edb5e35f9fc08
    821B miniscript miniscript::miniscript::astelem::<impl core::fmt::Debug for miniscript::miniscript::decode::Terminal<Pk,Ctx>>::fmt::h61cdb06e2a061932
    821B miniscript miniscript::miniscript::astelem::<impl core::fmt::Debug for miniscript::miniscript::decode::Terminal<Pk,Ctx>>::fmt::h44e1ab5daacde0df
    821B miniscript miniscript::miniscript::astelem::<impl core::fmt::Debug for miniscript::miniscript::decode::Terminal<Pk,Ctx>>::fmt::h27891ccf9609c782
    821B miniscript miniscript::miniscript::astelem::<impl core::fmt::Debug for miniscript::miniscript::decode::Terminal<Pk,Ctx>>::fmt::h1809b9931776159e
    522B miniscript miniscript::miniscript::astelem::fmt_n::h415db78b67d772cd
    490B miniscript miniscript::miniscript::astelem::fmt_n::hf8ac504ddfc86052
    470B miniscript miniscript::miniscript::astelem::fmt_n::hfbe0436e5641cffa
    470B miniscript miniscript::miniscript::astelem::fmt_n::hdd5863737990a62e
    470B miniscript miniscript::miniscript::astelem::fmt_n::hb797dce829f805d9
    470B miniscript miniscript::miniscript::astelem::fmt_n::had403a08f4f5ff39
    470B miniscript miniscript::miniscript::astelem::fmt_n::ha39bb63e21144b17
    470B miniscript miniscript::miniscript::astelem::fmt_n::h81e886e1ed45cc03
    470B miniscript miniscript::miniscript::astelem::fmt_n::h44aaf8763f1d95fc
    470B miniscript miniscript::miniscript::astelem::fmt_n::h2211a90c2ae72238
    470B miniscript miniscript::miniscript::astelem::fmt_n::h161245acb1186b04
    470B miniscript miniscript::miniscript::astelem::fmt_n::hf6086a767ac89cfd
    470B miniscript miniscript::miniscript::astelem::fmt_n::he64d26b7ff3d0e1c
    470B miniscript miniscript::miniscript::astelem::fmt_n::hd26b45ea2de7650c
    470B miniscript miniscript::miniscript::astelem::fmt_n::hd0626b47dacb58eb
    470B miniscript miniscript::miniscript::astelem::fmt_n::hd017036189a7590f
    470B miniscript miniscript::miniscript::astelem::fmt_n::hce30ab82918fd2f4
    470B miniscript miniscript::miniscript::astelem::fmt_n::hc929d63c76ef7f32
    470B miniscript miniscript::miniscript::astelem::fmt_n::hbbde06f3d3917d97
    470B miniscript miniscript::miniscript::astelem::fmt_n::hb5676374469e65b6
    470B miniscript miniscript::miniscript::astelem::fmt_n::hb4c4004b652070b6
    470B miniscript miniscript::miniscript::astelem::fmt_n::hb3c72f04d10f85c3
    470B miniscript miniscript::miniscript::astelem::fmt_n::h9228009d86e08451
    470B miniscript miniscript::miniscript::astelem::fmt_n::h863bfb226ca84db6
    470B miniscript miniscript::miniscript::astelem::fmt_n::h7f7484678076e46e
    470B miniscript miniscript::miniscript::astelem::fmt_n::h455f4bfd74ab437e
    470B miniscript miniscript::miniscript::astelem::fmt_n::h4193c28e8dff1cbd
    470B miniscript miniscript::miniscript::astelem::fmt_n::h3d7a7ebfc283c803
    470B miniscript miniscript::miniscript::astelem::fmt_n::h320c931de74d5eb5
    470B miniscript miniscript::miniscript::astelem::fmt_n::h0e6add3dbbd23ae8
    470B miniscript miniscript::miniscript::astelem::fmt_n::h0b5a7f7a0d66b692
    429B miniscript miniscript::miniscript::astelem::fmt_n::h4c35e66cc242cb0a
    429B miniscript miniscript::miniscript::astelem::fmt_n::hb0d356d143682d2b
    389B miniscript miniscript::miniscript::astelem::fmt_n::h0cd0b2cd4262a941
    389B miniscript miniscript::miniscript::astelem::fmt_n::he8cf072ceca64dcf
    352B miniscript miniscript::miniscript::astelem::fmt_n::h64eb21a37c79361f
    352B miniscript miniscript::miniscript::astelem::fmt_n::h63b4c672191846cc
    341B miniscript miniscript::miniscript::astelem::fmt_type_map::h5a67f7307d54634c
    324B miniscript miniscript::miniscript::astelem::fmt_n::h8bff9e8efc052ffd
    304B miniscript miniscript::miniscript::astelem::fmt_2::hd4ef6b5c2f834048
    304B miniscript miniscript::miniscript::astelem::fmt_2::hc7a021401a772d2f
    304B miniscript miniscript::miniscript::astelem::fmt_2::hb6b4382242ac534d
    304B miniscript miniscript::miniscript::astelem::fmt_2::hb002009b983f374f
    304B miniscript miniscript::miniscript::astelem::fmt_2::h79848b78c101cb36
    304B miniscript miniscript::miniscript::astelem::fmt_2::h58d79899838eb218
    304B miniscript miniscript::miniscript::astelem::fmt_2::h57c9081aea0c901d
    304B miniscript miniscript::miniscript::astelem::fmt_2::h381c281a2699a657
    304B miniscript miniscript::miniscript::astelem::fmt_2::h1ad912587742dd82
    304B miniscript miniscript::miniscript::astelem::fmt_2::hfbed77787659a86b
    304B miniscript miniscript::miniscript::astelem::fmt_2::he1956b794e6fccb3
    304B miniscript miniscript::miniscript::astelem::fmt_2::he0607b3c05ca9cd8
    304B miniscript miniscript::miniscript::astelem::fmt_2::hdde198aba2afed1b
    304B miniscript miniscript::miniscript::astelem::fmt_2::hd8e8123a831a66ab
    304B miniscript miniscript::miniscript::astelem::fmt_2::hcc96f49686aff28a
    304B miniscript miniscript::miniscript::astelem::fmt_2::hc427d84160c3ab50
    304B miniscript miniscript::miniscript::astelem::fmt_2::hb7985002b2623042
    304B miniscript miniscript::miniscript::astelem::fmt_2::ha7151690af4afc32
    304B miniscript miniscript::miniscript::astelem::fmt_2::h66fd3a4e3aff90d6
    304B miniscript miniscript::miniscript::astelem::fmt_2::h5c047e986c07b009
    304B miniscript miniscript::miniscript::astelem::fmt_2::h5afe1dca398d5653
    304B miniscript miniscript::miniscript::astelem::fmt_2::h5944bb12d6ad3730
    304B miniscript miniscript::miniscript::astelem::fmt_2::h57079cf38b86af02
    304B miniscript miniscript::miniscript::astelem::fmt_2::h510dae4ddf5e2a37
    304B miniscript miniscript::miniscript::astelem::fmt_2::h402972ca8b59b5ce
    304B miniscript miniscript::miniscript::astelem::fmt_2::h38a9fe8a553caea2
    304B miniscript miniscript::miniscript::astelem::fmt_2::h279ee0e7f3481d92
    304B miniscript miniscript::miniscript::astelem::fmt_2::h157c596525fb8aa2
    304B miniscript miniscript::miniscript::astelem::fmt_2::h077024e0f135605a
    301B miniscript miniscript::miniscript::astelem::fmt_1::h9dc876fc4bdf9e6d
    301B miniscript miniscript::miniscript::astelem::fmt_1::hb6f7b28b99e495e1
    294B miniscript miniscript::miniscript::astelem::fmt_1::h40e1215fb5a49a80
    294B miniscript miniscript::miniscript::astelem::fmt_1::h30bf641f8fa70c8a
    294B miniscript miniscript::miniscript::astelem::fmt_1::hcdf3279ef13f8db8
    294B miniscript miniscript::miniscript::astelem::fmt_1::ha8721e78f126f0a1
    276B miniscript miniscript::miniscript::astelem::fmt_1::h655aeaa3ba9a9945
    276B miniscript miniscript::miniscript::astelem::fmt_1::h646479614273ec91
    247B miniscript miniscript::miniscript::astelem::fmt_1::h19f7f43f7106f1ba
    204B miniscript miniscript::miniscript::astelem::fmt_1::hc69cb869e8ca59ad
    184B miniscript miniscript::miniscript::astelem::fmt_1::hd974a21946240094
    184B miniscript miniscript::miniscript::astelem::fmt_1::ha70df4fc9f667053
    127B miniscript miniscript::miniscript::astelem::fmt_1::ha04c6e79df58391a
    127B miniscript miniscript::miniscript::astelem::fmt_1::h6591abcf42b98a15
    127B miniscript miniscript::miniscript::astelem::fmt_1::hb023e179121fc2ca
    127B miniscript miniscript::miniscript::astelem::fmt_1::h0b24b9a2ae608c79
    115B miniscript miniscript::miniscript::astelem::conditional_fmt::hff0f003e7d2ff8f0
    115B miniscript miniscript::miniscript::astelem::conditional_fmt::hb26669d926903fd0
    115B miniscript miniscript::miniscript::astelem::conditional_fmt::h9167af6a2d7df04f
    115B miniscript miniscript::miniscript::astelem::conditional_fmt::h8ae550625ec0bbed
    115B miniscript miniscript::miniscript::astelem::conditional_fmt::h86454d296ba1f64e
    115B miniscript miniscript::miniscript::astelem::conditional_fmt::h84782d78834456f0
    115B miniscript miniscript::miniscript::astelem::conditional_fmt::h3d2876b270fe7a92
    115B miniscript miniscript::miniscript::astelem::conditional_fmt::h16df38e669c06c4b
    115B miniscript miniscript::miniscript::astelem::conditional_fmt::h0b92c426c268022d
    115B miniscript miniscript::miniscript::astelem::conditional_fmt::hfdd1f87e79efe630
    115B miniscript miniscript::miniscript::astelem::conditional_fmt::hf50802f369c01ae0
    115B miniscript miniscript::miniscript::astelem::conditional_fmt::he5bf1199456bc6ce
    115B miniscript miniscript::miniscript::astelem::conditional_fmt::he0e8357cc8ad2097
    115B miniscript miniscript::miniscript::astelem::conditional_fmt::hdf96384fef774987
    115B miniscript miniscript::miniscript::astelem::conditional_fmt::hdd6959a5f1851d9f
    115B miniscript miniscript::miniscript::astelem::conditional_fmt::hc62f1647f62ce39f
    115B miniscript miniscript::miniscript::astelem::conditional_fmt::ha33b8c3985af3cf0
    115B miniscript miniscript::miniscript::astelem::conditional_fmt::h924ec59e284f8f76
    115B miniscript miniscript::miniscript::astelem::conditional_fmt::h8bcd0f9caf7e07ba
    115B miniscript miniscript::miniscript::astelem::conditional_fmt::h79f3af696b89fedd
    115B miniscript miniscript::miniscript::astelem::conditional_fmt::h5e5892b8f2c195dd
    115B miniscript miniscript::miniscript::astelem::conditional_fmt::h45401eede6ac11de
    115B miniscript miniscript::miniscript::astelem::conditional_fmt::h4480ec61d9a4fb2b
    115B miniscript miniscript::miniscript::astelem::conditional_fmt::h3ca96698d40319aa
    115B miniscript miniscript::miniscript::astelem::conditional_fmt::h1fb3b0c5389b2440
    115B miniscript miniscript::miniscript::astelem::conditional_fmt::h184fd6275d931344
    115B miniscript miniscript::miniscript::astelem::conditional_fmt::h0d971005cd40179c
    115B miniscript miniscript::miniscript::astelem::conditional_fmt::h0d0ab3c5b3e4969e
    115B miniscript miniscript::miniscript::astelem::conditional_fmt::h0b976a0b97488a93
     91B miniscript miniscript::miniscript::astelem::fmt_1::h8a68544b05f3945f
      7B miniscript miniscript::miniscript::astelem::<impl core::fmt::Display for miniscript::miniscript::decode::Terminal<Pk,Ctx>>::fmt::hc779f37cb81f761a
      7B miniscript miniscript::miniscript::astelem::<impl core::fmt::Display for miniscript::miniscript::decode::Terminal<Pk,Ctx>>::fmt::hc4ac4a9b598fbdfb
      7B miniscript miniscript::miniscript::astelem::<impl core::fmt::Display for miniscript::miniscript::decode::Terminal<Pk,Ctx>>::fmt::h9986ae4314d85479
      7B miniscript miniscript::miniscript::astelem::<impl core::fmt::Display for miniscript::miniscript::decode::Terminal<Pk,Ctx>>::fmt::h82a27926db80dc37
      7B miniscript miniscript::miniscript::astelem::<impl core::fmt::Display for miniscript::miniscript::decode::Terminal<Pk,Ctx>>::fmt::h74343d637646e38a
      7B miniscript miniscript::miniscript::astelem::<impl core::fmt::Display for miniscript::miniscript::decode::Terminal<Pk,Ctx>>::fmt::h680da8fa0ec0142c
      7B miniscript miniscript::miniscript::astelem::<impl core::fmt::Display for miniscript::miniscript::decode::Terminal<Pk,Ctx>>::fmt::h6197658ba03f1fdf
      7B miniscript miniscript::miniscript::astelem::<impl core::fmt::Display for miniscript::miniscript::decode::Terminal<Pk,Ctx>>::fmt::h5cca31d73f30c3c7
      7B miniscript miniscript::miniscript::astelem::<impl core::fmt::Display for miniscript::miniscript::decode::Terminal<Pk,Ctx>>::fmt::h4588410e445d16c6
      7B miniscript miniscript::miniscript::astelem::<impl core::fmt::Display for miniscript::miniscript::decode::Terminal<Pk,Ctx>>::fmt::hfd9775e798913648
      7B miniscript miniscript::miniscript::astelem::<impl core::fmt::Display for miniscript::miniscript::decode::Terminal<Pk,Ctx>>::fmt::hf87563509ea7253d
      7B miniscript miniscript::miniscript::astelem::<impl core::fmt::Display for miniscript::miniscript::decode::Terminal<Pk,Ctx>>::fmt::heb60f683c9e55e36
      7B miniscript miniscript::miniscript::astelem::<impl core::fmt::Display for miniscript::miniscript::decode::Terminal<Pk,Ctx>>::fmt::hc12b3098c3979175
      7B miniscript miniscript::miniscript::astelem::<impl core::fmt::Display for miniscript::miniscript::decode::Terminal<Pk,Ctx>>::fmt::hbe7939a797fcce35
      7B miniscript miniscript::miniscript::astelem::<impl core::fmt::Display for miniscript::miniscript::decode::Terminal<Pk,Ctx>>::fmt::hb8b75c9c988d95c3
      7B miniscript miniscript::miniscript::astelem::<impl core::fmt::Display for miniscript::miniscript::decode::Terminal<Pk,Ctx>>::fmt::ha536af3ff4313ed1
      7B miniscript miniscript::miniscript::astelem::<impl core::fmt::Display for miniscript::miniscript::decode::Terminal<Pk,Ctx>>::fmt::h961e1a4b75732a11
      7B miniscript miniscript::miniscript::astelem::<impl core::fmt::Display for miniscript::miniscript::decode::Terminal<Pk,Ctx>>::fmt::h8969ddab726dea7f
      7B miniscript miniscript::miniscript::astelem::<impl core::fmt::Display for miniscript::miniscript::decode::Terminal<Pk,Ctx>>::fmt::h87b650968eb119ba
      7B miniscript miniscript::miniscript::astelem::<impl core::fmt::Display for miniscript::miniscript::decode::Terminal<Pk,Ctx>>::fmt::h7fca0d80a428dd24
      7B miniscript miniscript::miniscript::astelem::<impl core::fmt::Display for miniscript::miniscript::decode::Terminal<Pk,Ctx>>::fmt::h76e624d7abc545ba
      7B miniscript miniscript::miniscript::astelem::<impl core::fmt::Display for miniscript::miniscript::decode::Terminal<Pk,Ctx>>::fmt::h6b4909db760a4721
      7B miniscript miniscript::miniscript::astelem::<impl core::fmt::Display for miniscript::miniscript::decode::Terminal<Pk,Ctx>>::fmt::h5ed643a399966d49
      7B miniscript miniscript::miniscript::astelem::<impl core::fmt::Display for miniscript::miniscript::decode::Terminal<Pk,Ctx>>::fmt::h57b3cd148371177a
      7B miniscript miniscript::miniscript::astelem::<impl core::fmt::Display for miniscript::miniscript::decode::Terminal<Pk,Ctx>>::fmt::h24511846d004e114
      7B miniscript miniscript::miniscript::astelem::<impl core::fmt::Display for miniscript::miniscript::decode::Terminal<Pk,Ctx>>::fmt::h1ed7d512c859a5a9
      7B miniscript miniscript::miniscript::astelem::<impl core::fmt::Display for miniscript::miniscript::decode::Terminal<Pk,Ctx>>::fmt::h13d43f019a96d314
      7B miniscript miniscript::miniscript::astelem::<impl core::fmt::Display for miniscript::miniscript::decode::Terminal<Pk,Ctx>>::fmt::h0d96e9d943f04d7d
      7B miniscript miniscript::miniscript::astelem::<impl core::fmt::Display for miniscript::miniscript::decode::Terminal<Pk,Ctx>>::fmt::h008ca416c1ebc0d7
      0B            And 0 smaller methods. Use -n N to show more.
304.1KiB            filtered data size, the file size is 8.7MiB

@RCasatta
add conditional formatting for Terminal
426c34d 
fmt functions produce big binaries.
Since Debug and Display implementation of the match are equal except the Debug
or Display representation of its child, this introduce conditional fmt
trading performance for code size.
The branch introduced should be easily predicted so the performance impact
shouldn't be big.
@RCasatta RCasatta marked this pull request as ready for review March 5, 2024 17:37
@apoelstra
Copy link
Member

@RCasatta the reason is that in Display we implement the "aliases" pkh = c:pk_h and pk = c:pk_k.

I think it's fine to unify them so they both act like Display.

@RCasatta
apply immediate dispatch in generic fn
752acb3 
by refactoring out the function when the type is known, we avoid the duplication
in the generic function.

In the process write_char is replaced with write_str when the string is
static because by looking at the impl it saves a utf8 str conversion from the
char.
@RCasatta
unify debug/display impl for Terminal
395aab8
@RCasatta
Copy link
Contributor Author

RCasatta commented Mar 6, 2024

I think it's fine to unify them so they both act like Display.

Added a commit for this, updated failing test to the different Debug impl. There is less information printed out not sure it is desired.

Latest binary sizes didn't consider rebase on master, updated one are the following:
master 265.8KiB
1st commit 222.7KiB
2nd commit 215.5KiB
3rd commit 200.5KiB

@apoelstra
Copy link
Member

Cool, let's bring this in!

I have another idea to introduce an ErasedTerminal type which is basically just the enum without hashes or pubkeys, and then passing through to that in most cases. But that should wait for another PR since I think it's more general than formatting.

Another idea is to pull the wrapper-formatting logic out of the generic format. Though that maybe depends on ErasedTerminal too.

Anyway this is an improvement and it's not API-visible so it's fine to have some churn here. So let's merge it.

apoelstra
apoelstra approved these changes Mar 7, 2024
View reviewed changes
Copy link
Member

@apoelstra apoelstra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK 395aab8

@apoelstra apoelstra merged commit 22dad5e into rust-bitcoin:master Mar 7, 2024
16 checks passed
@RCasatta
Copy link
Contributor Author

RCasatta commented Mar 7, 2024

Probably similar to the ErasedTerminal idea, I was thinking that having a "hierarchical enum" grouping similar terminals together would allow some benefits, but I am not sure it's worth the complexity

apoelstra added a commit to apoelstra/rust-miniscript that referenced this pull request Mar 12, 2024
@apoelstra
clippy: fix some nits introduced by rust-bitcoin#651
b2ec4a8
apoelstra added a commit that referenced this pull request Mar 12, 2024
@apoelstra
Merge #657: compiler: improve logic when deciding between conjunction…
f6ffc3e 
…s and multi/multi_a

200991d compiler: improve logic for deciding between thresholds and ands (Andrew Poelstra)
bc3b8dc policy: rename Threshold variant to Thresh (Andrew Poelstra)
78db616 consolidate some FIXMEs (Andrew Poelstra)
0666aef error: remove some unused variants (Andrew Poelstra)
b2ec4a8 clippy: fix some nits introduced by #651 (Andrew Poelstra)

Pull request description:

  The compiler logic when encountering thresholds of pks is currently a bit confused, and therefore chooses multi/multi_a in cases where this is not the most efficient compilation.

  This PR fixes that, and also brings in a few other cleanup commits that I had laying around.

  This does **not** fix #656, which I didn't know how to approach.

ACKs for top commit:
  sanket1729:
    ACK 200991d

Tree-SHA512: 252a60891cf1c1d1cd3ded88d97122fd1e76bd25807770f4843ae68bd2d854fc617518f26be86dcb57cd7fc369e1a4be81daa42ee1a6d4bc976dbad6dc1150f6
pythcoiner pushed a commit to pythcoiner/rust-miniscript that referenced this pull request Aug 27, 2024
@apoelstra @pythcoiner
clippy: fix some nits introduced by rust-bitcoin#651
0361a25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@apoelstra apoelstra apoelstra approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@RCasatta @apoelstra