Skip to content

Commit

Permalink
Inline ASTNode bindings dependencies and observers (#15098)
Browse files Browse the repository at this point in the history
Every `ASTNode` contains two arrays used for type inference and checking: dependencies and observers. By default, these are created lazily, but most active (ie. effectively typed) `ASTNode`s end up creating them. Furthermore, on average both these lists contain less than 2 elements each.

This PR replaces both `Array(ASTNode)?` references in `ASTNode` by inlined structs that can hold two elements and a tail array for the cases where more links are needed. This reduces the number of allocations, bytes allocated, number of instructions executed and running time.

Some numbers from compiling the Crystal compiler itself without running codegen (since type binding occurs in the semantic phase anyway).

* Running time (measured with hyperfine running with `GC_DONT_GC=1`): ~6% reduction
  Before:
  ```
  Benchmark 1: ./self-semantic-only.sh
    Time (mean ± σ):      3.398 s ±  0.152 s    [User: 2.264 s, System: 0.470 s]
    Range (min … max):    3.029 s …  3.575 s    10 runs
  ```

  After:
  ```
  Benchmark 1: ./self-semantic-only.sh
    Time (mean ± σ):      3.180 s ±  0.095 s    [User: 2.153 s, System: 0.445 s]
    Range (min … max):    3.046 s …  3.345 s    10 runs
  ```

* Memory (as reported by the compiler itself, with GC): ~9.6% reduction
  Before:
  ```
  Parse:                             00:00:00.000038590 (   1.05MB)
  Semantic (top level):              00:00:00.483357706 ( 174.13MB)
  Semantic (new):                    00:00:00.002156811 ( 174.13MB)
  Semantic (type declarations):      00:00:00.038313066 ( 174.13MB)
  Semantic (abstract def check):     00:00:00.014283169 ( 190.13MB)
  Semantic (restrictions augmenter): 00:00:00.010672651 ( 206.13MB)
  Semantic (ivars initializers):     00:00:04.660611385 (1250.07MB)
  Semantic (cvars initializers):     00:00:00.008343907 (1250.07MB)
  Semantic (main):                   00:00:00.780627942 (1346.07MB)
  Semantic (cleanup):                00:00:00.000961914 (1346.07MB)
  Semantic (recursive struct check): 00:00:00.001121766 (1346.07MB)
  ```

  After:
  ```
  Parse:                             00:00:00.000044417 (   1.05MB)
  Semantic (top level):              00:00:00.546445955 ( 190.03MB)
  Semantic (new):                    00:00:00.002488975 ( 190.03MB)
  Semantic (type declarations):      00:00:00.040234541 ( 206.03MB)
  Semantic (abstract def check):     00:00:00.015473723 ( 222.03MB)
  Semantic (restrictions augmenter): 00:00:00.010828366 ( 222.03MB)
  Semantic (ivars initializers):     00:00:03.324639987 (1135.96MB)
  Semantic (cvars initializers):     00:00:00.007359853 (1135.96MB)
  Semantic (main):                   00:00:01.806822202 (1217.96MB)
  Semantic (cleanup):                00:00:00.000626975 (1217.96MB)
  Semantic (recursive struct check): 00:00:00.001435494 (1217.96MB)
  ```

* Callgrind stats:
  - Instruction refs: 17,477,865,704 -> 16,712,610,033 (~4.4% reduction)
  - Estimated cycles: 26,835,733,874 -> 26,154,926,143 (~2.5% reduction)
  - `GC_malloc_kind` call count: 35,161,616 -> 25,684,997 (~27% reduction)

Co-authored-by: Oleh Prypin <[email protected]>
Co-authored-by: Johannes Müller <[email protected]>
  • Loading branch information
3 people authored Oct 23, 2024
1 parent 8237397 commit 9d8c2e4
Show file tree
Hide file tree
Showing 6 changed files with 97 additions and 38 deletions.
111 changes: 86 additions & 25 deletions src/compiler/crystal/semantic/bindings.cr
Original file line number Diff line number Diff line change
@@ -1,7 +1,77 @@
module Crystal
# Specialized container for ASTNodes to use for bindings tracking.
#
# The average number of elements in both dependencies and observers is below 2
# for ASTNodes. This struct inlines the first two elements saving up 4
# allocations per node (two arrays, with a header and buffer for each) but we
# need to pay a slight extra cost in memory upfront: a total of 6 pointers (48
# bytes) vs 2 pointers (16 bytes). The other downside is that since this is a
# struct, we need to be careful with mutation.
struct SmallNodeList
include Enumerable(ASTNode)

@first : ASTNode?
@second : ASTNode?
@tail : Array(ASTNode)?

def each(& : ASTNode ->)
yield @first || return
yield @second || return
@tail.try(&.each { |node| yield node })
end

def size
if @first.nil?
0
elsif @second.nil?
1
elsif (tail = @tail).nil?
2
else
2 + tail.size
end
end

def push(node : ASTNode) : self
if @first.nil?
@first = node
elsif @second.nil?
@second = node
elsif (tail = @tail).nil?
@tail = [node] of ASTNode
else
tail.push(node)
end
self
end

def reject!(& : ASTNode ->) : self
if first = @first
if second = @second
if tail = @tail
tail.reject! { |node| yield node }
end
if yield second
@second = tail.try &.shift?
end
end
if yield first
@first = @second
@second = tail.try &.shift?
end
end
self
end

def concat(nodes : Enumerable(ASTNode)) : self
nodes.each { |node| self.push(node) }
self
end
end

class ASTNode
property! dependencies : Array(ASTNode)
property observers : Array(ASTNode)?
getter dependencies : SmallNodeList = SmallNodeList.new
@observers : SmallNodeList = SmallNodeList.new
property enclosing_call : Call?

@dirty = false
Expand Down Expand Up @@ -107,17 +177,17 @@ module Crystal
end

def bind_to(node : ASTNode) : Nil
bind(node) do |dependencies|
dependencies.push node
bind(node) do
@dependencies.push node
node.add_observer self
end
end

def bind_to(nodes : Indexable) : Nil
return if nodes.empty?

bind do |dependencies|
dependencies.concat nodes
bind do
@dependencies.concat nodes
nodes.each &.add_observer self
end
end
Expand All @@ -130,9 +200,7 @@ module Crystal
raise_frozen_type freeze_type, from_type, from
end

dependencies = @dependencies ||= [] of ASTNode

yield dependencies
yield

new_type = type_from_dependencies
new_type = map_type(new_type) if new_type
Expand All @@ -150,26 +218,25 @@ module Crystal
end

def type_from_dependencies : Type?
Type.merge dependencies
Type.merge @dependencies
end

def unbind_from(nodes : Nil)
# Nothing to do
end

def unbind_from(node : ASTNode)
@dependencies.try &.reject! &.same?(node)
@dependencies.reject! &.same?(node)
node.remove_observer self
end

def unbind_from(nodes : Array(ASTNode))
@dependencies.try &.reject! { |dep| nodes.any? &.same?(dep) }
def unbind_from(nodes : Enumerable(ASTNode))
@dependencies.reject! { |dep| nodes.any? &.same?(dep) }
nodes.each &.remove_observer self
end

def add_observer(observer)
observers = @observers ||= [] of ASTNode
observers.push observer
@observers.push observer
end

def remove_observer(observer)
Expand Down Expand Up @@ -269,16 +336,10 @@ module Crystal
visited = Set(ASTNode).new.compare_by_identity
owner_trace << node if node.type?.try &.includes_type?(owner)
visited.add node
while deps = node.dependencies?
dependencies = deps.select { |dep| dep.type? && dep.type.includes_type?(owner) && !visited.includes?(dep) }
if dependencies.size > 0
node = dependencies.first
nil_reason = node.nil_reason if node.is_a?(MetaTypeVar)
owner_trace << node if node
visited.add node
else
break
end
while node = node.dependencies.find { |dep| dep.type? && dep.type.includes_type?(owner) && !visited.includes?(dep) }
nil_reason = node.nil_reason if node.is_a?(MetaTypeVar)
owner_trace << node if node
visited.add node
end

MethodTraceException.new(owner, owner_trace, nil_reason, program.show_error_trace?)
Expand Down
3 changes: 1 addition & 2 deletions src/compiler/crystal/semantic/call_error.cr
Original file line number Diff line number Diff line change
Expand Up @@ -643,8 +643,7 @@ class Crystal::Call
if obj.is_a?(InstanceVar)
scope = self.scope
ivar = scope.lookup_instance_var(obj.name)
deps = ivar.dependencies?
if deps && deps.size == 1 && deps.first.same?(program.nil_var)
if ivar.dependencies.size == 1 && ivar.dependencies.first.same?(program.nil_var)
similar_name = scope.lookup_similar_instance_var_name(ivar.name)
if similar_name
msg << colorize(" (#{ivar.name} was never assigned a value, did you mean #{similar_name}?)").yellow.bold
Expand Down
5 changes: 1 addition & 4 deletions src/compiler/crystal/semantic/cleanup_transformer.cr
Original file line number Diff line number Diff line change
Expand Up @@ -1090,10 +1090,7 @@ module Crystal
node = super

unless node.type?
if dependencies = node.dependencies?
node.unbind_from node.dependencies
end

node.unbind_from node.dependencies
node.bind_to node.expressions
end

Expand Down
2 changes: 1 addition & 1 deletion src/compiler/crystal/semantic/filters.cr
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
module Crystal
class TypeFilteredNode < ASTNode
def initialize(@filter : TypeFilter, @node : ASTNode)
@dependencies = [@node] of ASTNode
@dependencies.push @node
node.add_observer self
update(@node)
end
Expand Down
4 changes: 2 additions & 2 deletions src/compiler/crystal/semantic/main_visitor.cr
Original file line number Diff line number Diff line change
Expand Up @@ -373,7 +373,7 @@ module Crystal
var.bind_to(@program.nil_var)
var.nil_if_read = false

meta_var.bind_to(@program.nil_var) unless meta_var.dependencies.try &.any? &.same?(@program.nil_var)
meta_var.bind_to(@program.nil_var) unless meta_var.dependencies.any? &.same?(@program.nil_var)
node.bind_to(@program.nil_var)
end

Expand Down Expand Up @@ -1283,7 +1283,7 @@ module Crystal
# It can happen that this call is inside an ArrayLiteral or HashLiteral,
# was expanded but isn't bound to the expansion because the call (together
# with its expansion) was cloned.
if (expanded = node.expanded) && (!node.dependencies? || !node.type?)
if (expanded = node.expanded) && (node.dependencies.empty? || !node.type?)
node.bind_to(expanded)
end

Expand Down
10 changes: 6 additions & 4 deletions src/compiler/crystal/semantic/type_merge.cr
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,18 @@ module Crystal
end
end

def type_merge(nodes : Array(ASTNode)) : Type?
def type_merge(nodes : Enumerable(ASTNode)) : Type?
case nodes.size
when 0
nil
when 1
nodes.first.type?
when 2
# Merging two types is the most common case, so we optimize it
first, second = nodes
type_merge(first.type?, second.type?)
# We use `#each_cons_pair` to avoid any intermediate allocation
nodes.each_cons_pair do |first, second|
return type_merge(first.type?, second.type?)
end
else
combined_union_of compact_types(nodes, &.type?)
end
Expand Down Expand Up @@ -161,7 +163,7 @@ module Crystal
end

class Type
def self.merge(nodes : Array(ASTNode)) : Type?
def self.merge(nodes : Enumerable(ASTNode)) : Type?
nodes.find(&.type?).try &.type.program.type_merge(nodes)
end

Expand Down

0 comments on commit 9d8c2e4

Please sign in to comment.