Inline some small functions

[jameshurst]

By adding the #[inline] attribute to these one line functions we can improve performance by quite a bit.

Before:

test bench_quick_xml            ... bench:     693,300 ns/iter (+/- 144,792)
test bench_quick_xml_escaped    ... bench:     832,738 ns/iter (+/- 449,167)
test bench_quick_xml_namespaced ... bench:     924,916 ns/iter (+/- 293,450)

After:

test bench_quick_xml            ... bench:     635,208 ns/iter (+/- 120,462)
test bench_quick_xml_escaped    ... bench:     791,582 ns/iter (+/- 366,737)
test bench_quick_xml_namespaced ... bench:     837,721 ns/iter (+/- 267,540)

[tafia]

Nice!
I don't think they are all necessary though.
What are the results with only is_whitespace and is_match for instance? Inlining pub fn in particular is generally not recommended.

[jameshurst]

With just inlining is_whitespace and is_match I got the following results:

test bench_quick_xml            ... bench:     691,151 ns/iter (+/- 388,130)
test bench_quick_xml_escaped    ... bench:     801,461 ns/iter (+/- 270,450)
test bench_quick_xml_namespaced ... bench:     876,823 ns/iter (+/- 211,925)

I believe for small functions like those one-liners and factory functions inlining a pub fn should be fine. I know that #[derive(Default)] autogenerates an #[inline] function for example.

[jameshurst]

If we just wanted to inline local functions then with inlining Element::from_buffer I get the following results:

test bench_quick_xml            ... bench:     623,252 ns/iter (+/- 41,585)
test bench_quick_xml_escaped    ... bench:     784,874 ns/iter (+/- 324,735)
test bench_quick_xml_namespaced ... bench:     858,635 ns/iter (+/- 377,527)

It looks like its actually faster for some benchmarks with just those three inlined than with what I submitted in this pull request.

[tafia]

Good! I'd prefer that indeed, there is not much to gain from inlining pub
fn and I'm afraid of optimising the benches and not real cases.
Can you push your changes ?
Thanks a lot!

On 27 Aug 2016 10:03, "James Hurst" notifications@github.com wrote:

If we just wanted to inline local functions then with inlining
Element::from_buffer I get the following results:

test bench_quick_xml ... bench: 624,222 ns/iter (+/- 130,794)
test bench_quick_xml_escaped ... bench: 804,864 ns/iter (+/- 510,050)
test bench_quick_xml_namespaced ... bench: 858,271 ns/iter (+/- 153,932)

—
You are receiving this because you commented.

Reply to this email directly, view it on GitHub
#38 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AHAsziTCLGFim8EdzYnUYulK_R8IRXYRks5qj5sDgaJpZM4JucAM
.

[jameshurst]

As for only optimizing only those benchmarks in the benches and not real cases. Here are the speeds I get in my library's benchmarks with the three inlines:

running 2 tests
test bench_extensions ... bench:   2,048,312 ns/iter (+/- 182,467)
test bench_rss2sample ... bench:      16,739 ns/iter (+/- 10,225)

With the version on crates.io I get:

test bench_extensions ... bench:   2,099,208 ns/iter (+/- 727,898)
test bench_rss2sample ... bench:      18,121 ns/iter (+/- 9,350)

[tafia]

I'm not with my computer, I'll publish a new version on crates.io asap.

[jameshurst]

Sounds good! 👍 If you could test out #37 before you push the new version that'd be great.

[tafia]

#37 merged and published v0.4.0.