Skip to content

Commit c6ae2c0

Browse files
skipkayhilskipkayhil
committed
ZJIT: improve array read-modify-write performance
With three optimizations: - constraining the effects of ArrayAref{,Checked} which could prevent load/store optimizations - keeping an Array's flags cached after ArrayAset instead of requiring them to be re-loaded - not emitting a WriteBarrier when its value is statically known to be an Immediate (previously they'd be removed in the fold_constants pass, but now optimization passes before fold_constants may be able to do more without having to run twice) I didn't see much change in ruby-bench benchmarks, but this sped up a math heavy gem I'm working on by 8.5%.
1 parent ae7ff74 commit c6ae2c0

3 files changed

Lines changed: 54 additions & 51 deletions

File tree

‎zjit/src/cruby_methods.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -474,7 +474,7 @@ fn inline_array_aset(fun: &mut hir::Function, block: hir::BlockId, recv: hir::In
474474
let index = fun.push_insn(block, hir::Insn::GuardGreaterEq { left: index, right: zero, reason: Box::new(SideExitReason::GuardGreaterEq), state });
475475

476476
let _ = fun.push_insn(block, hir::Insn::ArrayAset { array: recv, index, val });
477-
fun.push_insn(block, hir::Insn::WriteBarrier { recv, val });
477+
fun.push_write_barrier(block, recv, val);
478478
return Some(val);
479479
}
480480
}
@@ -1009,7 +1009,7 @@ fn inline_struct_initialize(fun: &mut hir::Function, block: hir::BlockId, recv:
10091009
match args.get(index as usize) {
10101010
Some(&val) => {
10111011
fun.push_insn(block, hir::Insn::StoreField { recv, id, offset, val, num_bits });
1012-
fun.push_insn(block, hir::Insn::WriteBarrier { recv, val });
1012+
fun.push_write_barrier(block, recv, val);
10131013
}
10141014
// nil is an immediate, so the tail needs no write barrier.
10151015
None => {

‎zjit/src/hir.rs‎

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1834,8 +1834,8 @@ impl Insn {
18341834
Insn::DupArrayInclude { .. } => effects::Any,
18351835
Insn::ArrayExtend { .. } => effects::Any,
18361836
Insn::ArrayPush { .. } => effects::Any,
1837-
Insn::ArrayAref { .. } => effects::Any,
1838-
Insn::ArrayArefChecked { .. } => effects::Any,
1837+
Insn::ArrayAref { .. } => Effect::read_write(abstract_heaps::Memory, abstract_heaps::Empty),
1838+
Insn::ArrayArefChecked { .. } => Effect::read_write(abstract_heaps::Memory, abstract_heaps::Empty),
18391839
Insn::ArrayAset { .. } => effects::Any,
18401840
Insn::ArrayPop { .. } => effects::Any,
18411841
Insn::ArrayLength { .. } => Effect::write(abstract_heaps::Empty),
@@ -3260,6 +3260,12 @@ impl Function {
32603260
id
32613261
}
32623262

3263+
pub fn push_write_barrier(&mut self, block: BlockId, recv: InsnId, val: InsnId) {
3264+
if !self.is_a(val, types::Immediate) {
3265+
self.push_insn(block, Insn::WriteBarrier { recv, val });
3266+
}
3267+
}
3268+
32633269
pub fn prepend_insn(&mut self, block: BlockId, insn: Insn) -> InsnId {
32643270
assert!(!matches!(insn, Insn::Param), "Cannot prepend a Param instruction");
32653271
let id = self.new_insn(insn);
@@ -5214,7 +5220,7 @@ impl Function {
52145220

52155221
let replacement = if let (OptimizedMethodType::StructAset, &[val]) = (opt_type, args.as_slice()) {
52165222
self.push_insn(block, Insn::StoreField { recv: target, id: mid.into(), offset, val, num_bits: types::BasicObject.num_bits() });
5217-
self.push_insn(block, Insn::WriteBarrier { recv, val });
5223+
self.push_write_barrier(block, recv, val);
52185224
val
52195225
} else { // StructAref
52205226
self.load_field(block, target, mid.into(), offset, types::BasicObject)
@@ -6348,7 +6354,7 @@ impl Function {
63486354
};
63496355

63506356
self.push_insn(block, Insn::StoreField { recv: ivar_storage, id: id.into(), offset, val, num_bits: types::BasicObject.num_bits() });
6351-
self.push_insn(block, Insn::WriteBarrier { recv: ivar_storage, val });
6357+
self.push_write_barrier(block, ivar_storage, val);
63526358
if spec.next_shape != spec.profiled_type.shape() {
63536359
// Write the new shape ID
63546360
let shape_id = self.push_insn(block, Insn::Const { val: Const::CShape(spec.next_shape) });
@@ -6729,6 +6735,13 @@ impl Function {
67296735
}
67306736
insn_id
67316737
}
6738+
&Insn::ArrayAset { .. } => {
6739+
// An element store never changes the array's flags, so keep cached flag
6740+
// loads but not other fields.
6741+
let offset = RUBY_OFFSET_RBASIC_FLAGS;
6742+
compile_time_heap.retain(|(_, off), _| *off == offset);
6743+
insn_id
6744+
}
67326745
&Insn::WriteBarrier { .. } => {
67336746
// Currently, WriteBarrier write effects are Allocator and Memory when we'd really like them to be flags.
67346747
// We don't use LoadField for mark bits so we can ignore them for now.

‎zjit/src/hir/opt_tests.rs‎

Lines changed: 35 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -13130,8 +13130,8 @@ mod hir_opt_tests {
1313013130
v32:CShape = LoadField v30, :shape_id@0x1040
1313113131
v33:CShape[0x1041] = GuardBitEquals v32, CShape(0x1041)
1313213132
StoreField v30, :@foo@0x1042, v17
13133-
v36:CShape[0x1043] = Const CShape(0x1043)
13134-
StoreField v30, :shape_id@0x1040, v36
13133+
v35:CShape[0x1043] = Const CShape(0x1043)
13134+
StoreField v30, :shape_id@0x1040, v35
1313513135
PatchPoint NoEPEscape(test)
1313613136
CheckInterrupts
1313713137
Return v17
@@ -13169,8 +13169,8 @@ mod hir_opt_tests {
1316913169
v32:CShape = LoadField v30, :shape_id@0x1040
1317013170
v33:CShape[0x1041] = GuardBitEquals v32, CShape(0x1041)
1317113171
StoreField v30, :@foo@0x1042, v17
13172-
v36:CShape[0x1043] = Const CShape(0x1043)
13173-
StoreField v30, :shape_id@0x1040, v36
13172+
v35:CShape[0x1043] = Const CShape(0x1043)
13173+
StoreField v30, :shape_id@0x1040, v35
1317413174
PatchPoint NoEPEscape(test)
1317513175
CheckInterrupts
1317613176
Return v17
@@ -13353,9 +13353,9 @@ mod hir_opt_tests {
1335313353
StoreField v50, :a@0x1069, v15
1335413354
StoreField v50, :b@0x106a, v17
1335513355
PatchPoint MethodRedefined(C@0x1008, b@0x106b, cme:0x1070)
13356-
v68:BasicObject = LoadField v50, :b@0x106a
13356+
v66:BasicObject = LoadField v50, :b@0x106a
1335713357
CheckInterrupts
13358-
Return v68
13358+
Return v66
1335913359
");
1336013360
}
1336113361

@@ -14664,9 +14664,9 @@ mod hir_opt_tests {
1466414664
v36:CUInt64 = LoadField v35, :RBASIC_FLAGS@0x1040
1466514665
v37:CUInt64 = GuardNoBitsSet v36, RUBY_FL_FREEZE=CUInt64(2048)
1466614666
v39:CUInt64 = GuardNoBitsSet v37, RUBY_ELTS_SHARED=CUInt64(4096)
14667-
v48:CInt64[1] = Const CInt64(1)
14667+
v47:CInt64[1] = Const CInt64(1)
1466814668
v41:CInt64 = ArrayLength v35
14669-
v42:CInt64[1] = GuardLess v48, v41
14669+
v42:CInt64[1] = GuardLess v47, v41
1467014670
ArrayAset v35, v42, v19
1467114671
PatchPoint NoEPEscape(test)
1467214672
CheckInterrupts
@@ -27077,16 +27077,12 @@ mod hir_opt_tests {
2707727077
v89:CInt64[0] = Const CInt64(0)
2707827078
v65:CInt64 = ArrayLength v63
2707927079
v67:BasicObject = ArrayArefChecked v63, v89, v65
27080-
PatchPoint NoEPEscape(test)
2708127080
PatchPoint MethodRedefined(Integer@0x1080, +@0x1088, cme:0x1090)
2708227081
v71:Fixnum = GuardType v67, Fixnum
2708327082
v72:Fixnum = FixnumAdd v59, v71
2708427083
v36:Fixnum[1] = Const Value(1)
27085-
PatchPoint NoSingletonClass(String@0x1008)
27086-
PatchPoint MethodRedefined(String@0x1008, getbyte@0x1010, cme:0x1018)
2708727084
v90:CInt64[1] = Const CInt64(1)
27088-
v78:CInt64 = LoadField v52, :len@0x1040
27089-
v79:CInt64[1] = GuardLess v90, v78
27085+
v79:CInt64[1] = GuardLess v90, v54
2709027086
v83:Fixnum = StringGetbyte v52, v79
2709127087
v87:Fixnum = FixnumAdd v72, v83
2709227088
CheckInterrupts
@@ -27125,22 +27121,21 @@ mod hir_opt_tests {
2712527121
v48:CUInt64 = LoadField v47, :RBASIC_FLAGS@0x1040
2712627122
v49:CUInt64 = GuardNoBitsSet v48, RUBY_FL_FREEZE=CUInt64(2048)
2712727123
v51:CUInt64 = GuardNoBitsSet v49, RUBY_ELTS_SHARED=CUInt64(4096)
27128-
v76:CInt64[0] = Const CInt64(0)
27124+
v74:CInt64[0] = Const CInt64(0)
2712927125
v53:CInt64 = ArrayLength v47
27130-
v54:CInt64[0] = GuardLess v76, v53
27126+
v54:CInt64[0] = GuardLess v74, v53
2713127127
ArrayAset v47, v54, v17
2713227128
PatchPoint NoEPEscape(test)
2713327129
v29:Fixnum[1] = Const Value(1)
2713427130
v31:Fixnum[2] = Const Value(2)
2713527131
PatchPoint NoSingletonClass(Array@0x1008)
2713627132
PatchPoint MethodRedefined(Array@0x1008, []=@0x1010, cme:0x1018)
27137-
v64:CUInt64 = LoadField v47, :RBASIC_FLAGS@0x1040
27138-
v65:CUInt64 = GuardNoBitsSet v64, RUBY_FL_FREEZE=CUInt64(2048)
27139-
v67:CUInt64 = GuardNoBitsSet v65, RUBY_ELTS_SHARED=CUInt64(4096)
27140-
v77:CInt64[1] = Const CInt64(1)
27141-
v69:CInt64 = ArrayLength v47
27142-
v70:CInt64[1] = GuardLess v77, v69
27143-
ArrayAset v47, v70, v31
27133+
v64:CUInt64 = GuardNoBitsSet v49, RUBY_FL_FREEZE=CUInt64(2048)
27134+
v66:CUInt64 = GuardNoBitsSet v49, RUBY_ELTS_SHARED=CUInt64(4096)
27135+
v75:CInt64[1] = Const CInt64(1)
27136+
v68:CInt64 = ArrayLength v47
27137+
v69:CInt64[1] = GuardLess v75, v68
27138+
ArrayAset v47, v69, v31
2714427139
PatchPoint NoEPEscape(test)
2714527140
CheckInterrupts
2714627141
Return v31
@@ -27187,7 +27182,6 @@ mod hir_opt_tests {
2718727182
PatchPoint MethodRedefined(Integer@0x1040, +@0x1048, cme:0x1050)
2718827183
v77:Fixnum = GuardType v74, Fixnum recompile
2718927184
v78:Fixnum = FixnumAdd v77, v28
27190-
PatchPoint NoSingletonClass(Array@0x1008)
2719127185
PatchPoint MethodRedefined(Array@0x1008, []=@0x1078, cme:0x1080)
2719227186
v84:CUInt64 = LoadField v69, :RBASIC_FLAGS@0x10a8
2719327187
v85:CUInt64 = GuardNoBitsSet v84, RUBY_FL_FREEZE=CUInt64(2048)
@@ -27203,31 +27197,28 @@ mod hir_opt_tests {
2720327197
v40:NilClass = Const Value(nil)
2720427198
PatchPoint NoSingletonClass(Array@0x1008)
2720527199
PatchPoint MethodRedefined(Array@0x1008, []@0x1010, cme:0x1018)
27206-
v100:Fixnum = GuardType v16, Fixnum
27207-
v101:CInt64 = UnboxFixnum v100
27208-
v102:CInt64 = ArrayLength v69
27209-
v103:CInt64 = AdjustBounds v101, v102
27210-
v104:BasicObject = ArrayArefChecked v69, v103, v102
27211-
PatchPoint NoEPEscape(test)
27200+
v99:Fixnum = GuardType v16, Fixnum
27201+
v100:CInt64 = UnboxFixnum v99
27202+
v101:CInt64 = ArrayLength v69
27203+
v102:CInt64 = AdjustBounds v100, v101
27204+
v103:BasicObject = ArrayArefChecked v69, v102, v101
2721227205
v50:Fixnum[1] = Const Value(1)
2721327206
PatchPoint MethodRedefined(Integer@0x1040, +@0x1048, cme:0x1050)
27214-
v107:Fixnum = GuardType v104, Fixnum recompile
27215-
v108:Fixnum = FixnumAdd v107, v50
27216-
PatchPoint NoSingletonClass(Array@0x1008)
27207+
v106:Fixnum = GuardType v103, Fixnum recompile
27208+
v107:Fixnum = FixnumAdd v106, v50
2721727209
PatchPoint MethodRedefined(Array@0x1008, []=@0x1078, cme:0x1080)
27218-
v114:CUInt64 = LoadField v69, :RBASIC_FLAGS@0x10a8
27219-
v115:CUInt64 = GuardNoBitsSet v114, RUBY_FL_FREEZE=CUInt64(2048)
27220-
v117:CUInt64 = GuardNoBitsSet v115, RUBY_ELTS_SHARED=CUInt64(4096)
27221-
v118:CInt64 = UnboxFixnum v100
27222-
v119:CInt64 = ArrayLength v69
27223-
v120:CInt64 = GuardLess v118, v119
27224-
v121:CInt64 = AdjustBounds v120, v119
27225-
v122:CInt64[0] = Const CInt64(0)
27226-
v123:CInt64 = GuardGreaterEq v121, v122
27227-
ArrayAset v69, v123, v108
27210+
v114:CUInt64 = GuardNoBitsSet v85, RUBY_FL_FREEZE=CUInt64(2048)
27211+
v116:CUInt64 = GuardNoBitsSet v85, RUBY_ELTS_SHARED=CUInt64(4096)
27212+
v117:CInt64 = UnboxFixnum v99
27213+
v118:CInt64 = ArrayLength v69
27214+
v119:CInt64 = GuardLess v117, v118
27215+
v120:CInt64 = AdjustBounds v119, v118
27216+
v121:CInt64[0] = Const CInt64(0)
27217+
v122:CInt64 = GuardGreaterEq v120, v121
27218+
ArrayAset v69, v122, v107
2722827219
PatchPoint NoEPEscape(test)
2722927220
CheckInterrupts
27230-
Return v108
27221+
Return v107
2723127222
");
2723227223
}
2723327224

@@ -27322,8 +27313,7 @@ mod hir_opt_tests {
2732227313
PatchPoint NoEPEscape(test)
2732327314
v26:Fixnum[2] = Const Value(2)
2732427315
PatchPoint MethodRedefined(S@0x1008, b=@0x1042, cme:0x1048)
27325-
v48:CUInt64 = LoadField v40, :RBASIC_FLAGS@0x1040
27326-
v49:CUInt64 = GuardNoBitsSet v48, RUBY_FL_FREEZE=CUInt64(2048)
27316+
v48:CUInt64 = GuardNoBitsSet v42, RUBY_FL_FREEZE=CUInt64(2048)
2732727317
StoreField v40, :b=@0x1070, v26
2732827318
CheckInterrupts
2732927319
Return v26

0 commit comments

Comments
 (0)