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

derive: escape strings at compile-time when possible #92

Merged
merged 4 commits into from
Jul 28, 2024

Conversation

Kijewski
Copy link
Collaborator

@Kijewski Kijewski commented Jul 27, 2024

Todo:

  • look if there is a proper string parsing function
  • benchmark derive

@Kijewski Kijewski changed the title derive: compile strings at compile-time when possible derive: escape strings at compile-time when possible Jul 27, 2024
@Kijewski Kijewski force-pushed the pr-escape-at-compile-time branch 5 times, most recently from 8d6b595 to f90e561 Compare July 27, 2024 04:21
@Kijewski
Copy link
Collaborator Author

derive benchmark
hello_world             time:   [50.551 µs 50.673 µs 50.795 µs]
                        change: [+0.2145% +0.5681% +0.9316%] (p = 0.00 < 0.05)
                        Change within noise threshold.

item_info.html          time:   [60.100 µs 60.221 µs 60.354 µs]
                        change: [+0.7116% +0.9772% +1.2335%] (p = 0.00 < 0.05)
                        Change within noise threshold.

item_union.html         time:   [187.10 µs 187.49 µs 187.92 µs]
                        change: [+3.3728% +3.8026% +4.2713%] (p = 0.00 < 0.05)
                        Performance has regressed.

page.html               time:   [777.26 µs 777.97 µs 778.76 µs]
                        change: [-0.5346% -0.0855% +0.5532%] (p = 0.83 > 0.05)
                        No change in performance detected.

print_item.html         time:   [154.32 µs 154.46 µs 154.62 µs]
                        change: [-0.6921% -0.4250% -0.1376%] (p = 0.00 < 0.05)
                        Change within noise threshold.

short_item_info.html    time:   [129.97 µs 130.19 µs 130.43 µs]
                        change: [+1.5539% +1.9741% +2.4175%] (p = 0.00 < 0.05)
                        Performance has regressed.

sidebar.html            time:   [200.02 µs 200.28 µs 200.56 µs]
                        change: [+2.1864% +2.4044% +2.6125%] (p = 0.00 < 0.05)
                        Performance has regressed.

source.html             time:   [116.60 µs 116.76 µs 116.92 µs]
                        change: [+3.0496% +3.3321% +3.6158%] (p = 0.00 < 0.05)
                        Performance has regressed.

type_layout.html        time:   [167.39 µs 167.57 µs 167.78 µs]
                        change: [+3.4081% +4.8191% +5.7045%] (p = 0.00 < 0.05)
                        Performance has regressed.

type_layout_size.html   time:   [67.252 µs 67.361 µs 67.470 µs]
                        change: [+1.2518% +1.6146% +1.9632%] (p = 0.00 < 0.05)
                        Performance has regressed.

Acceptable.

@Kijewski Kijewski marked this pull request as ready for review July 27, 2024 18:32
@Kijewski Kijewski force-pushed the pr-escape-at-compile-time branch 2 times, most recently from ac68829 to 411d463 Compare July 28, 2024 07:30
@GuillaumeGomez
Copy link
Contributor

It's a great idea! I'm surprised the impact on compile-time is this little, but it makes it even better.

Thanks!

@GuillaumeGomez GuillaumeGomez merged commit 7408a98 into rinja-rs:master Jul 28, 2024
17 checks passed
@Kijewski Kijewski deleted the pr-escape-at-compile-time branch July 28, 2024 14:12
@Kijewski
Copy link
Collaborator Author

I'm surprised the impact on compile-time is this little, but it makes it even better.

The case is not actually used in our tests. In rustdoc's code which we use in out benchmarks, there is no {{ "…" }}. So we just messure the overhead of inspecting the Exprs before pushing them into the write queue, and fast dropping a Cow::Borrowed(…) is.

The "red and black suits" example comes from out of our tests, though.

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.

2 participants