-
Notifications
You must be signed in to change notification settings - Fork 5.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Profiling feature #6565
Profiling feature #6565
Changes from 6 commits
3b3f964
39cff93
60c8291
4478c59
cdcb082
48553f0
f422522
44f7102
4d78c6b
525cfe1
ebc0676
2a410f3
248098d
bc6a73e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -41,9 +41,14 @@ tracing = "0.1" | |
url = { version = "2.2", features = ["serde"] } | ||
vec1 = "1.8.0" | ||
walkdir = "2" | ||
fuel-asm = { version = "0.57.0", features = ["serde"] } | ||
sway-ir = { version = "0.63.6", path = "../sway-ir" } | ||
|
||
[dev-dependencies] | ||
regex = "^1.10.2" | ||
|
||
[target.'cfg(not(target_os = "macos"))'.dependencies] | ||
tritao marked this conversation as resolved.
Show resolved
Hide resolved
|
||
sysinfo = "0.29" | ||
|
||
[features] | ||
profile = [] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: I'd prefer to call it |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1785,6 +1785,7 @@ pub fn compile( | |
|
||
// First, compile to an AST. We'll update the namespace and check for JSON ABI output. | ||
let ast_res = time_expr!( | ||
pkg.name, | ||
"compile to ast", | ||
"compile_to_ast", | ||
sway_core::compile_to_ast( | ||
|
@@ -1823,6 +1824,7 @@ pub fn compile( | |
} | ||
|
||
let asm_res = time_expr!( | ||
pkg.name, | ||
"compile ast to asm", | ||
"compile_ast_to_asm", | ||
sway_core::ast_to_asm(&handler, engines, &programs, &sway_build_config), | ||
|
@@ -1837,6 +1839,7 @@ pub fn compile( | |
let mut program_abi = match pkg.target { | ||
BuildTarget::Fuel => { | ||
let program_abi_res = time_expr!( | ||
pkg.name, | ||
"generate JSON ABI program", | ||
"generate_json_abi", | ||
fuel_abi::generate_program_abi( | ||
|
@@ -1875,6 +1878,7 @@ pub fn compile( | |
}; | ||
|
||
let abi = time_expr!( | ||
pkg.name, | ||
"generate JSON ABI program", | ||
"generate_json_abi", | ||
evm_abi::generate_abi_program(typed_program, engines), | ||
|
@@ -1899,15 +1903,16 @@ pub fn compile( | |
.map(|finalized_entry| PkgEntry::from_finalized_entry(finalized_entry, engines)) | ||
.collect::<anyhow::Result<_>>()?; | ||
|
||
let asm = match asm_res { | ||
let mut asm = match asm_res { | ||
Err(_) => return fail(handler), | ||
Ok(asm) => asm, | ||
}; | ||
|
||
let bc_res = time_expr!( | ||
pkg.name, | ||
"compile asm to bytecode", | ||
"compile_asm_to_bytecode", | ||
sway_core::asm_to_bytecode(&handler, asm, source_map, engines.se(), &sway_build_config), | ||
sway_core::asm_to_bytecode(&handler, &mut asm, source_map, engines.se(), &sway_build_config), | ||
Some(sway_build_config.clone()), | ||
metrics | ||
); | ||
|
@@ -1957,9 +1962,62 @@ pub fn compile( | |
warnings, | ||
metrics, | ||
}; | ||
|
||
#[cfg(feature = "profile")] | ||
report_assembly_information(&asm, &compiled_package); | ||
|
||
Ok(compiled_package) | ||
} | ||
|
||
#[cfg(feature = "profile")] | ||
fn report_assembly_information( | ||
compiled_asm: &sway_core::CompiledAsm, | ||
compiled_package: &CompiledPackage, | ||
) { | ||
let mut bytes = compiled_package.bytecode.bytes.clone(); | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To future-proof, There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for pointing this out. We have implemented this for the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice, thanks. Sorry just one last thing - seeing that
And then deal with a possible error for the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We have taken a look at the data flow for the datum entries, and we have determined that it is the responsibility of the compiler to produce valid data section slot size values that sum within the range of a |
||
let data_offset = u64::from_be_bytes(bytes.iter().skip(8).take(8).cloned().collect::<Vec<_>>().try_into().unwrap()); | ||
let data_section_size = bytes.len() as u64 - data_offset; | ||
|
||
bytes.truncate(data_offset as usize); | ||
|
||
let mut data_section_used = 0u64; | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can we remove this empty new line? Sorry for my OCD here There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed. |
||
fn calculate_entry_size(entry: &sway_core::asm_generation::Entry) -> u64 { | ||
let padding = match &entry.padding { | ||
sway_ir::Padding::Left { target_size } => target_size, | ||
sway_ir::Padding::Right { target_size } => target_size, | ||
}; | ||
|
||
let value_size = match &entry.value { | ||
sway_core::asm_generation::Datum::Byte(x) => std::mem::size_of_val(x) as u64, | ||
sway_core::asm_generation::Datum::Word(x) => std::mem::size_of_val(x) as u64, | ||
sway_core::asm_generation::Datum::ByteArray(x) => std::mem::size_of_val(x) as u64, | ||
sway_core::asm_generation::Datum::Slice(slice) => slice.len() as u64, | ||
sway_core::asm_generation::Datum::Collection(entries) => entries.iter().map(calculate_entry_size).sum(), | ||
}; | ||
|
||
assert_eq!(*padding as u64, value_size); | ||
|
||
value_size | ||
} | ||
|
||
for entry in &compiled_asm.0.data_section.value_pairs { | ||
data_section_used += calculate_entry_size(entry); | ||
} | ||
|
||
let asm_information = sway_core::asm_generation::AsmInformation { | ||
bytecode_size: bytes.len() as _, | ||
data_section: sway_core::asm_generation::DataSectionInformation { | ||
size: data_section_size, | ||
used: data_section_used, | ||
value_pairs: compiled_asm.0.data_section.value_pairs.clone(), | ||
} | ||
}; | ||
|
||
println!("/forc-perf info {}", serde_json::to_string(&asm_information).unwrap()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
So it communicates with forc-perf over stdout? Could you add to the PR description links to forc-perf documentation, and how to test this locally? |
||
} | ||
|
||
impl PkgEntry { | ||
/// Returns whether this `PkgEntry` corresponds to a test. | ||
pub fn is_test(&self) -> bool { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -49,6 +49,7 @@ whoami = "1.1" | |
|
||
[features] | ||
default = [] | ||
profile = [] | ||
test = [] | ||
util = [] | ||
uwu = ["uwuify"] | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should these dependencies only be added for the profile feature?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for pointing these out. The
fuel-asm
crate is no longer being used and has been removed. Thesway_ir
crate is indeed only being used for the profiling functionality, so we have made this an optional dependency that only loads when using theprofiler
feature.