Skip to content

Commit

Permalink
Shrink Crystal::System.print_error's output size (#15490)
Browse files Browse the repository at this point in the history
`Crystal::System` is by far the single largest LLVM module when compiling a blank source file, even though all the module does by itself is defining `.print_error` and friends. On my machine, with debug information stripped, `C-rystal5858S-ystem.o0.bc` is 213.4 KiB big, compared to `_main.o0.bc`'s 101.7 KiB. Disassembling the bytecode back to LLVM IR produces a monstrosity with 33k lines. This PR brings the numbers down to 48.0 KiB and 6.1k lines, while slightly improving performance, using the following tricks:

* The type of `.as?(T)` is always `T?` and does not perform intersection, so even simple types like `Int32` are upcast into the whole `Int::Primitive?`, leading to a lot of redundant downcasts later. A simple `is_a?` will suffice as a type filter in `read_arg`. (I believe this is mentioned somewhere but couldn't find it)
* In `.to_int_slice`, the `num` variable is cast into an `Int32 | UInt32 | Int64 | UInt64`, and each subsequent line dispatches over that union. The fix here is to split the rest of the body into a separate method, and call it with each variant of the union. This form of dispatching is akin to rewriting `.to_int_slice` as an instance method on the integers.
* `.to_int_slice` is now non-yielding, as the inlining added too much bloat. The caller is responsible for preparing a suitably sized buffer.

Additionally, this reduces the time for the bytecode generation phase from an average of 0.35s down to 0.26s.
  • Loading branch information
HertzDevil authored Feb 22, 2025
1 parent 2998ccf commit 8d5e093
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 27 deletions.
53 changes: 29 additions & 24 deletions src/crystal/system/print_error.cr
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,12 @@ module Crystal::System
finish = ptr + format_len
arg_index = 0

# The widest integer types supported by the format specifier are `%lld` and
# `%llu`, which do not exceed 64 bits, so we only need 20 digits maximum
# note that `chars` does not have to be null-terminated, since we are
# only yielding a `Bytes`
int_chars = uninitialized UInt8[20]

while ptr < finish
next_percent = ptr
while next_percent < finish && !(next_percent.value === '%')
Expand Down Expand Up @@ -94,20 +100,20 @@ module Crystal::System
end
when 'd'
read_arg(Int::Primitive) do |arg|
to_int_slice(arg, 10, true, width) { |bytes| yield bytes }
yield to_int_slice(int_chars.to_slice, arg, 10, true, width)
end
when 'u'
read_arg(Int::Primitive) do |arg|
to_int_slice(arg, 10, false, width) { |bytes| yield bytes }
yield to_int_slice(int_chars.to_slice, arg, 10, false, width)
end
when 'x'
read_arg(Int::Primitive) do |arg|
to_int_slice(arg, 16, false, width) { |bytes| yield bytes }
yield to_int_slice(int_chars.to_slice, arg, 16, false, width)
end
when 'p'
read_arg(Pointer(Void)) do |arg|
yield "0x".to_slice
to_int_slice(arg.address, 16, false, 2) { |bytes| yield bytes }
yield to_int_slice(int_chars.to_slice, arg.address, 16, false, 2)
end
else
yield Slice.new(next_percent, fmt_ptr + 1 - next_percent)
Expand All @@ -118,8 +124,8 @@ module Crystal::System
end

private macro read_arg(type, &block)
{{ block.args[0] }} = args[arg_index].as?({{ type }})
if !{{ block.args[0] }}.nil?
{{ block.args[0] }} = args[arg_index]
if {{ block.args[0] }}.is_a?({{ type }})
{{ block.body }}
else
yield "(???)".to_slice
Expand All @@ -140,28 +146,27 @@ module Crystal::System
end

# simplified version of `Int#internal_to_s`
protected def self.to_int_slice(num, base, signed, width, &)
protected def self.to_int_slice(buf, num, base, signed, width)
if num == 0
yield "0".to_slice
return
"0".to_slice
else
# NOTE: do not factor out `num`! it is written this way to inhibit
# unnecessary union dispatches
case {signed, width}
when {true, 2} then to_int_slice_impl(buf, LibC::LongLong.new!(num), base)
when {true, 1} then to_int_slice_impl(buf, LibC::Long.new!(num), base)
when {true, 0} then to_int_slice_impl(buf, LibC::Int.new!(num), base)
when {false, 2} then to_int_slice_impl(buf, LibC::ULongLong.new!(num), base)
when {false, 1} then to_int_slice_impl(buf, LibC::ULong.new!(num), base)
else to_int_slice_impl(buf, LibC::UInt.new!(num), base)
end
end
end

# Given sizeof(num) <= 64 bits, we need at most 20 bytes for `%d` or `%u`
# note that `chars` does not have to be null-terminated, since we are
# only yielding a `Bytes`
chars = uninitialized UInt8[20]
ptr_end = chars.to_unsafe + 20
private def self.to_int_slice_impl(buf, num, base)
ptr_end = buf.to_unsafe + buf.size
ptr = ptr_end

num = case {signed, width}
when {true, 2} then LibC::LongLong.new!(num)
when {true, 1} then LibC::Long.new!(num)
when {true, 0} then LibC::Int.new!(num)
when {false, 2} then LibC::ULongLong.new!(num)
when {false, 1} then LibC::ULong.new!(num)
else LibC::UInt.new!(num)
end

neg = num < 0

# do not assume Crystal constant initialization succeeds, hence not `DIGITS`
Expand All @@ -178,7 +183,7 @@ module Crystal::System
ptr.value = '-'.ord.to_u8
end

yield Slice.new(ptr, ptr_end - ptr)
Slice.new(ptr, ptr_end - ptr)
end

def self.print_exception(message, ex)
Expand Down
7 changes: 4 additions & 3 deletions src/crystal/tracing.cr
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ module Crystal

def initialize
@buf = uninitialized UInt8[N]
@int_buf = uninitialized UInt8[20] # max 64-bit integers
@size = 0
end

Expand Down Expand Up @@ -83,15 +84,15 @@ module Crystal

def write(value : Pointer) : Nil
write "0x"
System.to_int_slice(value.address, 16, true, 2) { |bytes| write(bytes) }
write System.to_int_slice(@int_buf.to_slice, value.address, 16, true, 2)
end

def write(value : Int::Signed) : Nil
System.to_int_slice(value, 10, true, 2) { |bytes| write(bytes) }
write System.to_int_slice(@int_buf.to_slice, value, 10, true, 2)
end

def write(value : Int::Unsigned) : Nil
System.to_int_slice(value, 10, false, 2) { |bytes| write(bytes) }
write System.to_int_slice(@int_buf.to_slice, value, 10, false, 2)
end

def write(value : Time::Span) : Nil
Expand Down

0 comments on commit 8d5e093

Please sign in to comment.