Optimize Integer decoding from bytes #11796
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
By using Pointer#copy_to and not allocation new slices.
|
There's actually no memory allocation involved here. This works slightly faster in this PR because some bound checks are removed, which makes this easily prone to segfault. I'm not sure we should do that. I'm not on the computer, but if I have time I will send code that makes this change segfault. |
|
Yes, the change to I believe the change to |
|
Removed |
straight-shoota
approved these changes
Feb 4, 2022
carlhoerberg
changed the title
beta-ziliani
approved these changes
Mar 18, 2022
|
@carlhoerberg Your benchmark results seem to have run into the issue where each iteration is so fast that the results are inaccurate. I updated the benchmark code to run more iterations inside the Updated coderequire "benchmark"
module IO::ByteFormat
{% for mod in %w(LittleEndian BigEndian) %}
module {{mod.id}}
{% for type, i in %w(Int8 UInt8 Int16 UInt16 Int32 UInt32 Int64 UInt64 Int128 UInt128) %}
{% bytesize = 2 ** (i // 2) %}
def self.decode2(type : {{type.id}}.class, bytes : Bytes)
buffer = uninitialized UInt8[{{bytesize}}]
bytes.copy_to(buffer.to_unsafe, {{bytesize}})
buffer.reverse! unless SystemEndian == self
buffer.unsafe_as({{type.id}})
end
{% end %}
end
{% end %}
end
b = Bytes.new(128) { 1u8 }
iterations = 1_000
{% for mod in %w(LittleEndian BigEndian) %}
{% for type, i in %w(Int8 UInt8 Int16 UInt16 Int32 UInt32 Int64 UInt64 Int128 UInt128) %}
Benchmark.ips do |x|
x.report("old decode {{mod.id}} {{ type.id }}") do
iterations.times { IO::ByteFormat::{{mod.id}}.decode({{type.id}}, b) }
end
x.report("new decode {{mod.id}} {{ type.id }}") do
iterations.times { IO::ByteFormat::{{mod.id}}.decode2({{type.id}}, b) }
end
end
{% end %}
{% end %}Diff from the original benchmark code19a20
> iterations = 1_000
25c26
< IO::ByteFormat::{{mod.id}}.decode({{type.id}}, b)
---
> iterations.times { IO::ByteFormat::{{mod.id}}.decode({{type.id}}, b) }
28c29
< IO::ByteFormat::{{mod.id}}.decode2({{type.id}}, b)
---
> iterations.times { IO::ByteFormat::{{mod.id}}.decode2({{type.id}}, b) }Intel resultsARM64 resultsSeems like LLVM might be optimizing the stack allocation out of the resulting binary. I'm surprised your results were so consistently one-sided. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
By using
Slice#copy_to(target : Pointer(T), count)and not (stack) allocating new slices.Benchmark
Results
Discussion
Decoding is about 80% faster with the notable of exception BigEndian (U)Int64, which becomes 15% slower.