Skip to content

Commit 53ea2d6

Browse files
committed
Use sparse sorted Vec for IseqProfile instead of dense pre-allocated Vecs
Replace the two dense Vec fields (opnd_types and num_profiles) that were pre-allocated to iseq_size for every ISEQ with a single sorted Vec<(u32, ProfileEntry)> that only allocates entries for instructions that actually get profiled. Lookups use binary search. On Lobsters benchmark: zjit_alloc_bytes drops from 38.9 MB to 20.7 MB (46.8% reduction) with no performance regression. Shrink Distribution counts from usize to u16 The counts in Distribution only need to track frequency during profiling (typically up to num_profiles=30). u16 (max 65535) is more than enough and saves 6 bytes per count slot. Distribution<ProfiledType, 4> shrinks from 104 to 74 bytes. On Lobsters: zjit_alloc_bytes drops from 20.7 MB to 18.5 MB. wip Revert "wip" This reverts commit a3fe018442fb267c9350dcdb027eddf30bcc08bb. ZJIT: Restore accidentally removed comments ZJIT: Remove unused iseq_size parameter from IseqProfile::new ZJIT: Move insn_idx into ProfileEntry struct ZJIT: Add per-rev result caching to zjit_diff and build with stats ZJIT: Split Distribution into Mono/Poly variants to reduce profile memory ZJIT: Shrink NumProfiles from u32 to u16 Revert "ZJIT: Split Distribution into Mono/Poly variants to reduce profile memory" This reverts commit 01a87703413f218d25973afd894b272edb51c12f. .
1 parent bb402ec commit 53ea2d6

5 files changed

Lines changed: 120 additions & 46 deletions

File tree

tool/zjit_diff.rb

Lines changed: 46 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
# frozen_string_literal: true
33

44
require 'fileutils'
5+
require 'json'
56
require 'optparse'
67
require 'tmpdir'
78
require 'logger'
@@ -11,6 +12,7 @@
1112
GitRef = Struct.new(:ref, :commit_hash)
1213

1314
RUBIES_DIR = File.join(Dir.home, '.zjit-diff')
15+
RESULTS_CACHE_DIR = File.join(Dir.home, '.zjit-diff', 'results')
1416
BEFORE_NAME = 'ruby-zjit-before'
1517
AFTER_NAME = 'ruby-zjit-after'
1618

@@ -51,17 +53,52 @@ def bench!
5153

5254
private
5355

54-
def run_benchmarks(ruby_bench_path)
56+
def results_cache_key(rev_hash)
57+
key_parts = [rev_hash, @options[:bench_args], @options[:name_filters]].inspect
58+
Digest::MD5.hexdigest(key_parts)
59+
end
60+
61+
def run_rev(ruby_bench_path, name, rev_hash)
62+
cache_file = File.join(RESULTS_CACHE_DIR, "#{results_cache_key(rev_hash)}.json")
63+
64+
if File.exist?(cache_file) && !@options[:force_rerun]
65+
LOG.info("Using cached results for #{name} from #{cache_file}")
66+
return JSON.parse(File.read(cache_file))
67+
end
68+
69+
out_name = File.join('data', "zjit_diff_#{name}")
5570
Dir.chdir(ruby_bench_path) do
5671
@runner.cmd({ 'RUBIES_DIR' => RUBIES_DIR },
5772
'./run_benchmarks.rb',
5873
'--chruby',
59-
"before::#{@before_hash} --zjit-stats;after::#{@after_hash} --zjit-stats",
74+
"#{name}::#{rev_hash} --zjit-stats",
6075
'--out-name',
61-
DATA_FILENAME,
76+
out_name,
6277
*@options[:bench_args],
6378
*@options[:name_filters])
79+
end
80+
81+
result = JSON.parse(File.read(File.join(ruby_bench_path, "#{out_name}.json")))
82+
FileUtils.mkdir_p(RESULTS_CACHE_DIR)
83+
File.write(cache_file, JSON.generate(result))
84+
LOG.info("Cached results for #{name} to #{cache_file}")
85+
result
86+
end
87+
88+
def run_benchmarks(ruby_bench_path)
89+
before_data = run_rev(ruby_bench_path, 'before', @before_hash)
90+
after_data = run_rev(ruby_bench_path, 'after', @after_hash)
91+
92+
merged = {
93+
'metadata' => before_data['metadata'].merge(after_data['metadata']),
94+
'raw_data' => before_data['raw_data'].merge(after_data['raw_data']),
95+
}
6496

97+
merged_path = File.join(ruby_bench_path, "#{DATA_FILENAME}.json")
98+
FileUtils.mkdir_p(File.dirname(merged_path))
99+
File.write(merged_path, JSON.generate(merged))
100+
101+
Dir.chdir(ruby_bench_path) do
65102
@runner.cmd('./misc/zjit_diff.rb', "#{DATA_FILENAME}.json", out: $stdout)
66103
end
67104
end
@@ -98,7 +135,7 @@ def initialize(name:, ref:, runner:, force_rebuild: false)
98135

99136
def build!
100137
Dir.chdir(@path) do
101-
configure_cmd_args = ['--enable-zjit=dev', '--disable-install-doc']
138+
configure_cmd_args = ['--enable-zjit=stats', '--disable-install-doc']
102139
if macos?
103140
brew_prefixes = BREW_REQUIRED_PACKAGES.map do |pkg|
104141
`brew --prefix #{pkg}`.strip
@@ -157,7 +194,7 @@ def clean!
157194
end
158195

159196
if Dir.exist?(RUBIES_DIR)
160-
LOG.info("Removing ruby installations from #{RUBIES_DIR}")
197+
LOG.info("Removing ruby installations and cached results from #{RUBIES_DIR}")
161198
FileUtils.rm_rf(RUBIES_DIR)
162199
end
163200

@@ -231,6 +268,10 @@ def parse_ref(ref)
231268
options[:force_rebuild] = true
232269
end
233270

271+
opts.on('--force-rerun', 'Force re-running benchmarks even if cached results exist') do
272+
options[:force_rerun] = true
273+
end
274+
234275
opts.on('--quiet', 'Silence output of commands except for benchmark result') do
235276
options[:quiet] = true
236277
end

zjit/src/distribution.rs

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
//! Type frequency distribution tracker.
22
3+
use crate::options::NumProfiles;
4+
35
/// This implementation was inspired by the type feedback module from Google's S6, which was
46
/// written in C++ for use with Python. This is a new implementation in Rust created for use with
57
/// Ruby instead of Python.
@@ -8,9 +10,9 @@ pub struct Distribution<T: Copy + PartialEq + Default, const N: usize> {
810
/// buckets and counts have the same length
911
/// `buckets[0]` is always the most common item
1012
buckets: [T; N],
11-
counts: [usize; N],
13+
counts: [NumProfiles; N],
1214
/// if there is no more room, increment the fallback
13-
other: usize,
15+
other: NumProfiles,
1416
// TODO(max): Add count disparity, which can help determine when to reset the distribution
1517
}
1618

@@ -23,13 +25,13 @@ impl<T: Copy + PartialEq + Default, const N: usize> Distribution<T, N> {
2325
for (bucket, count) in self.buckets.iter_mut().zip(self.counts.iter_mut()) {
2426
if *bucket == item || *count == 0 {
2527
*bucket = item;
26-
*count += 1;
28+
*count = count.saturating_add(1);
2729
// Keep the most frequent item at the front
2830
self.bubble_up();
2931
return;
3032
}
3133
}
32-
self.other += 1;
34+
self.other = self.other.saturating_add(1);
3335
}
3436

3537
/// Keep the highest counted bucket at index 0
@@ -87,7 +89,7 @@ impl<T: Copy + PartialEq + Default + std::fmt::Debug, const N: usize> Distributi
8789
assert!(first_count >= count, "First count should be the largest");
8890
}
8991
}
90-
let num_seen = dist.counts.iter().sum::<usize>() + dist.other;
92+
let num_seen = dist.counts.iter().map(|&c| c as usize).sum::<usize>() + dist.other as usize;
9193
let kind = if dist.other == 0 {
9294
// Seen <= N types total
9395
if dist.counts[0] == 0 {

zjit/src/options.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ use std::collections::HashSet;
88

99
/// Default --zjit-num-profiles
1010
const DEFAULT_NUM_PROFILES: NumProfiles = 5;
11-
pub type NumProfiles = u32;
11+
pub type NumProfiles = u16;
1212

1313
/// Default --zjit-call-threshold. This should be large enough to avoid compiling
1414
/// warmup code, but small enough to perform well on micro-benchmarks.

zjit/src/payload.rs

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,9 @@ pub struct IseqPayload {
1414
}
1515

1616
impl IseqPayload {
17-
fn new(iseq_size: u32) -> Self {
17+
fn new() -> Self {
1818
Self {
19-
profile: IseqProfile::new(iseq_size),
19+
profile: IseqProfile::new(),
2020
versions: vec![],
2121
}
2222
}
@@ -87,8 +87,7 @@ pub fn get_or_create_iseq_payload_ptr(iseq: IseqPtr) -> *mut IseqPayload {
8787
// We drop the payload with Box::from_raw when the GC frees the ISEQ and calls us.
8888
// NOTE(alan): Sometimes we read from an ISEQ without ever writing to it.
8989
// We allocate in those cases anyways.
90-
let iseq_size = get_iseq_encoded_size(iseq);
91-
let new_payload = IseqPayload::new(iseq_size);
90+
let new_payload = IseqPayload::new();
9291
let new_payload = Box::into_raw(Box::new(new_payload));
9392
rb_iseq_set_zjit_payload(iseq, new_payload as VoidPtr);
9493

zjit/src/profile.rs

Lines changed: 63 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -104,8 +104,9 @@ fn profile_insn(bare_opcode: ruby_vminsn_type, ec: EcPtr) {
104104
}
105105

106106
// Once we profile the instruction num_profiles times, we stop profiling it.
107-
profile.num_profiles[profiler.insn_idx] = profile.num_profiles[profiler.insn_idx].saturating_add(1);
108-
if profile.num_profiles[profiler.insn_idx] == get_option!(num_profiles) {
107+
let entry = profile.entry_mut(profiler.insn_idx);
108+
entry.num_profiles = entry.num_profiles.saturating_add(1);
109+
if entry.num_profiles == get_option!(num_profiles) {
109110
unsafe { rb_zjit_iseq_insn_set(profiler.iseq, profiler.insn_idx as u32, bare_opcode); }
110111
}
111112
}
@@ -118,12 +119,12 @@ pub type TypeDistributionSummary = DistributionSummary<ProfiledType, DISTRIBUTIO
118119

119120
/// Profile the Type of top-`n` stack operands
120121
fn profile_operands(profiler: &mut Profiler, profile: &mut IseqProfile, n: usize) {
121-
let types = &mut profile.opnd_types[profiler.insn_idx];
122-
if types.is_empty() {
123-
types.resize(n, TypeDistribution::new());
122+
let entry = profile.entry_mut(profiler.insn_idx);
123+
if entry.opnd_types.is_empty() {
124+
entry.opnd_types.resize(n, TypeDistribution::new());
124125
}
125126

126-
for (i, profile_type) in types.iter_mut().enumerate() {
127+
for (i, profile_type) in entry.opnd_types.iter_mut().enumerate() {
127128
let obj = profiler.peek_at_stack((n - i - 1) as isize);
128129
// TODO(max): Handle GC-hidden classes like Array, Hash, etc and make them look normal or
129130
// drop them or something
@@ -134,33 +135,33 @@ fn profile_operands(profiler: &mut Profiler, profile: &mut IseqProfile, n: usize
134135
}
135136

136137
fn profile_self(profiler: &mut Profiler, profile: &mut IseqProfile) {
137-
let types = &mut profile.opnd_types[profiler.insn_idx];
138-
if types.is_empty() {
139-
types.resize(1, TypeDistribution::new());
138+
let entry = profile.entry_mut(profiler.insn_idx);
139+
if entry.opnd_types.is_empty() {
140+
entry.opnd_types.resize(1, TypeDistribution::new());
140141
}
141142
let obj = profiler.peek_at_self();
142143
// TODO(max): Handle GC-hidden classes like Array, Hash, etc and make them look normal or
143144
// drop them or something
144145
let ty = ProfiledType::new(obj);
145146
VALUE::from(profiler.iseq).write_barrier(ty.class());
146-
types[0].observe(ty);
147+
entry.opnd_types[0].observe(ty);
147148
}
148149

149150
fn profile_block_handler(profiler: &mut Profiler, profile: &mut IseqProfile) {
150-
let types = &mut profile.opnd_types[profiler.insn_idx];
151-
if types.is_empty() {
152-
types.resize(1, TypeDistribution::new());
151+
let entry = profile.entry_mut(profiler.insn_idx);
152+
if entry.opnd_types.is_empty() {
153+
entry.opnd_types.resize(1, TypeDistribution::new());
153154
}
154155
let obj = profiler.peek_at_block_handler();
155156
let ty = ProfiledType::object(obj);
156157
VALUE::from(profiler.iseq).write_barrier(ty.class());
157-
types[0].observe(ty);
158+
entry.opnd_types[0].observe(ty);
158159
}
159160

160161
fn profile_getblockparamproxy(profiler: &mut Profiler, profile: &mut IseqProfile) {
161-
let types = &mut profile.opnd_types[profiler.insn_idx];
162-
if types.is_empty() {
163-
types.resize(1, TypeDistribution::new());
162+
let entry = profile.entry_mut(profiler.insn_idx);
163+
if entry.opnd_types.is_empty() {
164+
entry.opnd_types.resize(1, TypeDistribution::new());
164165
}
165166

166167
let level = profiler.insn_opnd(1).as_u32();
@@ -170,7 +171,7 @@ fn profile_getblockparamproxy(profiler: &mut Profiler, profile: &mut IseqProfile
170171

171172
let ty = ProfiledType::object(untagged);
172173
VALUE::from(profiler.iseq).write_barrier(ty.class());
173-
types[0].observe(ty);
174+
entry.opnd_types[0].observe(ty);
174175
}
175176

176177
fn profile_invokesuper(profiler: &mut Profiler, profile: &mut IseqProfile) {
@@ -374,30 +375,61 @@ impl ProfiledType {
374375
}
375376
}
376377

378+
/// Per-instruction profile entry, stored sparsely in a sorted Vec.
377379
#[derive(Debug)]
378-
pub struct IseqProfile {
379-
/// Type information of YARV instruction operands, indexed by the instruction index
380-
opnd_types: Vec<Vec<TypeDistribution>>,
380+
struct ProfileEntry {
381+
/// YARV instruction index
382+
insn_idx: u32,
383+
/// Type information of YARV instruction operands
384+
opnd_types: Vec<TypeDistribution>,
385+
/// Number of profiled executions for this YARV instruction
386+
num_profiles: NumProfiles,
387+
}
381388

382-
/// Number of profiled executions for each YARV instruction, indexed by the instruction index
383-
num_profiles: Vec<NumProfiles>,
389+
#[derive(Debug)]
390+
pub struct IseqProfile {
391+
/// Sparse storage of per-instruction profile data, sorted by instruction index.
392+
/// Only instructions that have actually been profiled have entries here.
393+
entries: Vec<ProfileEntry>,
384394

385395
/// Method entries for `super` calls (stored as VALUE to be GC-safe)
386396
super_cme: HashMap<usize, TypeDistribution>
387397
}
388398

389399
impl IseqProfile {
390-
pub fn new(iseq_size: u32) -> Self {
400+
pub fn new() -> Self {
391401
Self {
392-
opnd_types: vec![vec![]; iseq_size as usize],
393-
num_profiles: vec![0; iseq_size as usize],
402+
entries: Vec::new(),
394403
super_cme: HashMap::new(),
395404
}
396405
}
397406

407+
/// Get or create a mutable profile entry for the given instruction index.
408+
fn entry_mut(&mut self, insn_idx: usize) -> &mut ProfileEntry {
409+
let idx = insn_idx as u32;
410+
match self.entries.binary_search_by_key(&idx, |e| e.insn_idx) {
411+
Ok(i) => &mut self.entries[i],
412+
Err(i) => {
413+
self.entries.insert(i, ProfileEntry {
414+
insn_idx: idx,
415+
opnd_types: Vec::new(),
416+
num_profiles: 0,
417+
});
418+
&mut self.entries[i]
419+
}
420+
}
421+
}
422+
423+
/// Get a profile entry for the given instruction index (read-only).
424+
fn entry(&self, insn_idx: usize) -> Option<&ProfileEntry> {
425+
let idx = insn_idx as u32;
426+
self.entries.binary_search_by_key(&idx, |e| e.insn_idx)
427+
.ok().map(|i| &self.entries[i])
428+
}
429+
398430
/// Get profiled operand types for a given instruction index
399431
pub fn get_operand_types(&self, insn_idx: usize) -> Option<&[TypeDistribution]> {
400-
self.opnd_types.get(insn_idx).map(|v| &**v)
432+
self.entry(insn_idx).map(|e| e.opnd_types.as_slice()).filter(|s| !s.is_empty())
401433
}
402434

403435
pub fn get_super_method_entry(&self, insn_idx: usize) -> Option<*const rb_callable_method_entry_t> {
@@ -413,8 +445,8 @@ impl IseqProfile {
413445

414446
/// Run a given callback with every object in IseqProfile
415447
pub fn each_object(&self, callback: impl Fn(VALUE)) {
416-
for operands in &self.opnd_types {
417-
for distribution in operands {
448+
for entry in &self.entries {
449+
for distribution in &entry.opnd_types {
418450
for profiled_type in distribution.each_item() {
419451
// If the type is a GC object, call the callback
420452
callback(profiled_type.class);
@@ -431,8 +463,8 @@ impl IseqProfile {
431463

432464
/// Run a given callback with a mutable reference to every object in IseqProfile.
433465
pub fn each_object_mut(&mut self, callback: impl Fn(&mut VALUE)) {
434-
for operands in &mut self.opnd_types {
435-
for distribution in operands {
466+
for entry in &mut self.entries {
467+
for distribution in &mut entry.opnd_types {
436468
for ref mut profiled_type in distribution.each_item_mut() {
437469
// If the type is a GC object, call the callback
438470
callback(&mut profiled_type.class);

0 commit comments

Comments
 (0)