Skip to content
This repository has been archived by the owner on Jul 22, 2024. It is now read-only.

Add flamegraph target to Makefile #205

Merged
merged 18 commits into from
Mar 21, 2023
Merged

Add flamegraph target to Makefile #205

merged 18 commits into from
Mar 21, 2023

Conversation

MegaRedHand
Copy link
Contributor

@MegaRedHand MegaRedHand commented Mar 17, 2023

This PR adds a Makefile target that runs a copy of tests/deploy_account.rs with some differences to allow for better flamegraph visualization.

Note: also inverts the guard-check in this method of InternalDeploy.

@codecov-commenter
Copy link

codecov-commenter commented Mar 17, 2023

Codecov Report

Merging #205 (38637ad) into main (a5971a2) will decrease coverage by 2.54%.
The diff coverage is 1.06%.

@@            Coverage Diff             @@
##             main     #205      +/-   ##
==========================================
- Coverage   73.47%   70.93%   -2.54%     
==========================================
  Files          39       40       +1     
  Lines        2601     2694      +93     
==========================================
  Hits         1911     1911              
- Misses        690      783      +93     
Impacted Files Coverage Δ
bench/internals.rs 0.00% <0.00%> (ø)
src/services/api/contract_class.rs 87.65% <0.00%> (-3.38%) ⬇️
src/testing/starknet_state.rs 65.93% <ø> (ø)
...iness_logic/transaction/objects/internal_deploy.rs 52.50% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@MegaRedHand MegaRedHand marked this pull request as ready for review March 20, 2023 18:09
Comment on lines +26 to +29
static ref CONTRACT_CLASS: ContractClass = ContractClass::try_from(PathBuf::from(
"starknet_programs/account_without_validation.json",
))
.unwrap();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any way to build the ContractClass from bytes? In that case you could use include_bytes! instead of creating a path that needs to be read from.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The GH workflow that runs clippy doesn't install deps and so cannot compile the cairo programs. I'll revert this change.

To do so, had to add compile-starknet to Makefile target dependencies.
@mmsc2 mmsc2 merged commit e7193c7 into main Mar 21, 2023
@mmsc2 mmsc2 deleted the prof-flamegraph branch March 21, 2023 15:40
igamigo pushed a commit that referenced this pull request Mar 21, 2023
* Add flamegraph target to Makefile

* Add dependency to target

* Fix: activate venv before compiling contracts

* Rename benches

* Add declare test

- don't use test harness

* Clone elements outside `scope`

* Appease clippy

* Add deploy test

* Reduce run count

* Fix: invert guard in `handle_empty_constructor`

* Use correct class_hash in test

* Add invoke flamegraph

* Pin cargo-flamegraph version

* Add instructions to README

* Remove unnecessary clone

* Add ContractClass with include_str!

To do so, had to add compile-starknet to Makefile target dependencies.

* Revert "Add ContractClass with include_str!"

This reverts commit 10f9c96.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants