Project

General

Profile

Actions

Feature #17613

closed

Eliminate useless catch tables and nops from lambdas

Added by tenderlovemaking (Aaron Patterson) over 3 years ago. Updated over 3 years ago.

Status:
Closed
Assignee:
-
Target version:
-
[ruby-core:102418]

Description

This patch frees catch tables on iseqs that don't use the catch tables. It also eliminates nop instructions from lambdas that don't need them.

Before this patch, lambdas have a "prelude nop" that is used for catch table entries:

$ ruby --dump=insn -e '1.times { |x| puts x }'
== disasm: #<ISeq:<main>@-e:1 (1,0)-(1,22)> (catch: FALSE)
== catch table
| catch type: break  st: 0000 ed: 0004 sp: 0000 cont: 0004
| == disasm: #<ISeq:block in <main>@-e:1 (1,8)-(1,22)> (catch: FALSE)
| == catch table
| | catch type: redo   st: 0001 ed: 0006 sp: 0000 cont: 0001
| | catch type: next   st: 0001 ed: 0006 sp: 0000 cont: 0006
| |------------------------------------------------------------------------
| local table (size: 1, argc: 1 [opts: 0, rest: -1, post: 0, block: -1, kw: -1@-1, kwrest: -1])
| [ 1] x@0<Arg>
| 0000 nop                                                              (   1)[Bc]
| 0001 putself                                [Li]
| 0002 getlocal_WC_0                          x@0
| 0004 opt_send_without_block                 <calldata!mid:puts, argc:1, FCALL|ARGS_SIMPLE>
| 0006 leave                                  [Br]
|------------------------------------------------------------------------
0000 putobject_INT2FIX_1_                                             (   1)[Li]
0001 send                                   <calldata!mid:times, argc:0>, block in <main>
0004 leave

But since this particular lambda doesn't use the catch tables, there is no reason to keep the catch table or the nop instruction. This patch eliminates the nop instructions as well as the unused catch tables:

> ruby --dump=insn -e '1.times { |x| puts x }'
== disasm: #<ISeq:<main>@-e:1 (1,0)-(1,22)> (catch: FALSE)
0000 putobject_INT2FIX_1_                                             (   1)[Li]
0001 send                                   <calldata!mid:times, argc:0>, block in <main>
0004 leave

== disasm: #<ISeq:block in <main>@-e:1 (1,8)-(1,22)> (catch: FALSE)
local table (size: 1, argc: 1 [opts: 0, rest: -1, post: 0, block: -1, kw: -1@-1, kwrest: -1])
[ 1] x@0<Arg>
0000 putself                                                          (   1)[LiBc]
0001 getlocal_WC_0                          x@0
0003 opt_send_without_block                 <calldata!mid:puts, argc:1, FCALL|ARGS_SIMPLE>
0005 leave

It's not huge, but this frees about 600kb of catch tables on RailsBench. Here is a histogram of the catch tables and sizes freed for RailsBench:

Freed Catch Tables

The X axis is the catch table size, so the actually malloc'd size for 2 would be approximately 2 * sizeof(struct iseq_catch_table_entry). So if we have 5 tables of size 2, that would be about 5 * 2 * sizeof(struct iseq_catch_table_entry).

The size of iseq_catch_table_entry is 32:

(lldb) p sizeof(struct iseq_catch_table_entry)
(unsigned long) $0 = 32

The total catch tables freed in RailsBench is 18275, so this frees about 18275 * 32 bytes, or about 584kb:

> sum(freed_table_sizes$V1)
[1] 18275
> sum(freed_table_sizes$V1) * 32
[1] 584800

Instruction Sequence size is also reduced due to nop elimination, but I didn't measure it.

Finally, this patch reduces nop calls on RailsBench from 6868813 ( 2.1%) to 2467772 ( 0.8%).

nop instructions on the master branch (265c002239):

[RUBY_INSNS_COUNTER]	nop                                  6868813 ( 2.1%)

nop instructions with this patch applied:

[RUBY_INSNS_COUNTER]	nop                                  2467772 ( 0.8%)

Pull request is here


Files


Related issues 2 (0 open2 closed)

Related to Ruby master - Bug #18474: 938e027c seems to have caused a regression in yield handling with concurrent-rubyClosedActions
Related to Ruby master - Bug #18475: Yielding an element for Enumerator in another thread dumps coreClosedActions
Actions #1

Updated by tenderlovemaking (Aaron Patterson) over 3 years ago

  • Status changed from Open to Closed

Applied in changeset git|938e027cdf019ff2cb6ee8a7229e6d9a4d8fc953.


Eliminate useless catch tables and nops from lambdas

Before this commit:

$ ruby --dump=insn -e '1.times { |x| puts x }'
== disasm: #<ISeq:<main>@-e:1 (1,0)-(1,22)> (catch: FALSE)
== catch table
| catch type: break  st: 0000 ed: 0004 sp: 0000 cont: 0004
| == disasm: #<ISeq:block in <main>@-e:1 (1,8)-(1,22)> (catch: FALSE)
| == catch table
| | catch type: redo   st: 0001 ed: 0006 sp: 0000 cont: 0001
| | catch type: next   st: 0001 ed: 0006 sp: 0000 cont: 0006
| |------------------------------------------------------------------------
| local table (size: 1, argc: 1 [opts: 0, rest: -1, post: 0, block: -1, kw: -1@-1, kwrest: -1])
| [ 1] x@0<Arg>
| 0000 nop                                                              (   1)[Bc]
| 0001 putself                                [Li]
| 0002 getlocal_WC_0                          x@0
| 0004 opt_send_without_block                 <calldata!mid:puts, argc:1, FCALL|ARGS_SIMPLE>
| 0006 leave                                  [Br]
|------------------------------------------------------------------------
0000 putobject_INT2FIX_1_                                             (   1)[Li]
0001 send                                   <calldata!mid:times, argc:0>, block in <main>
0004 leave

After this commit:

> ruby --dump=insn -e '1.times { |x| puts x }'
== disasm: #<ISeq:<main>@-e:1 (1,0)-(1,22)> (catch: FALSE)
0000 putobject_INT2FIX_1_                                             (   1)[Li]
0001 send                                   <calldata!mid:times, argc:0>, block in <main>
0004 leave

== disasm: #<ISeq:block in <main>@-e:1 (1,8)-(1,22)> (catch: FALSE)
local table (size: 1, argc: 1 [opts: 0, rest: -1, post: 0, block: -1, kw: -1@-1, kwrest: -1])
[ 1] x@0<Arg>
0000 putself                                                          (   1)[LiBc]
0001 getlocal_WC_0                          x@0
0003 opt_send_without_block                 <calldata!mid:puts, argc:1, FCALL|ARGS_SIMPLE>
0005 leave

Fixes [ruby-core:102418] [Feature #17613]

Co-Authored-By: Alan Wu

Actions #2

Updated by mame (Yusuke Endoh) almost 3 years ago

  • Related to Bug #18474: 938e027c seems to have caused a regression in yield handling with concurrent-ruby added
Actions #3

Updated by mame (Yusuke Endoh) almost 3 years ago

  • Related to Bug #18475: Yielding an element for Enumerator in another thread dumps core added
Actions

Also available in: Atom PDF

Like0
Like0Like0Like0