Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

perf(render): Use a Write #192

Merged
merged 1 commit into from
Sep 6, 2018
Merged

perf(render): Use a Write #192

merged 1 commit into from
Sep 6, 2018

Conversation

epage
Copy link
Member

@epage epage commented Sep 5, 2018

render_to uses an io::Write. This is hopefully better than string
concatenation, especially if you can write directly to file. render is
still available for convinience (though it no longer returns an
Option).

Fixes #187

BREAKING CHANGES: Changed focus to io::Write

  • Renderables must implement render_to.
  • render no longer returns an Option.

@epage
Copy link
Member Author

epage commented Sep 5, 2018

Before:

test bench_parse_template ... bench: 25,826 ns/iter (+/- 1,689)
test bench_parse_text ... bench: 634 ns/iter (+/- 52)
test bench_parse_variable ... bench: 2,527 ns/iter (+/- 182)
test bench_render_template ... bench: 7,225 ns/iter (+/- 2,762)
test bench_render_text ... bench: 262 ns/iter (+/- 20)
test bench_render_variable ... bench: 1,392 ns/iter (+/- 112)

@epage
Copy link
Member Author

epage commented Sep 5, 2018

After

test bench_parse_template ... bench: 25,657 ns/iter (+/- 2,238)
test bench_parse_text ... bench: 629 ns/iter (+/- 37)
test bench_parse_variable ... bench: 2,562 ns/iter (+/- 203)
test bench_render_template ... bench: 7,600 ns/iter (+/- 726)
test bench_render_text ... bench: 248 ns/iter (+/- 105)
test bench_render_variable ... bench: 1,368 ns/iter (+/- 101)

Note: no real allocation change was made to the benchmark. It still calls render rather than render_to. In addition, it doesn't have many allocation heavy code paths.

`render_to` uses an `io::Write`.  This is hopefully better than string
concatenation, especially if you can write directly to file. `render` is
still available for convinience (though it no longer returns an
`Option`).

Fixes cobalt-org#187

BREAKING CHANGES: Changed focus to `io::Write`
- `Renderable`s must implement `render_to`.
- `render` no longer returns an `Option`.
@epage epage merged commit 9750f22 into cobalt-org:master Sep 6, 2018
@epage epage deleted the write branch September 6, 2018 00:02
@emoon
Copy link
Contributor

emoon commented Sep 6, 2018

Nice. Did you consider taking a generic parameter for Write? Now it's a trait object so that adds some extra overhead but perhaps it doesn't affect that much.

@epage
Copy link
Member Author

epage commented Sep 6, 2018

It wouldn't have worked with how we track Renderables. They are boxed, so we need an object-safe function which means dyn Write rather than impl Write.

@emoon
Copy link
Contributor

emoon commented Sep 6, 2018

Ah, I see

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants