Skip to content
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

Clean up profile data and vm outcome #4615

Closed
2 of 4 tasks
ailisp opened this issue Aug 2, 2021 · 6 comments
Closed
2 of 4 tasks

Clean up profile data and vm outcome #4615

ailisp opened this issue Aug 2, 2021 · 6 comments
Assignees
Labels
A-contract-runtime Area: contract compilation and execution, virtual machines, etc C-enhancement Category: An issue proposing an enhancement or a PR with one. T-contract-runtime Team: issues relevant to the contract runtime team

Comments

@ailisp
Copy link
Member

ailisp commented Aug 2, 2021

From @matklad 's comment in #4582:

  • remove serialize/deserialize from vm outcome
  • re-install partial eq on vm outcome
  • don't track burn gas in two places
  • get rid of the cells inside ProfileData
@bowenwang1996 bowenwang1996 added A-contract-runtime Area: contract compilation and execution, virtual machines, etc T-contract-runtime Team: issues relevant to the contract runtime team C-enhancement Category: An issue proposing an enhancement or a PR with one. labels Aug 2, 2021
@ailisp
Copy link
Member Author

ailisp commented Aug 3, 2021

Note: We need VMOutcome Serialize in standalone runner, so we can't drop it yet.

@ailisp
Copy link
Member Author

ailisp commented Aug 3, 2021

Hmm drop burn gas in ProfileData is also problematic. burn gas is the ProfileData.all_gas(), and profile data need it to figure out wasm_gas(). The other way would be drop burnt_gas from VMOutcome

@stale
Copy link

stale bot commented Nov 1, 2021

This issue has been automatically marked as stale because it has not had recent activity in the last 2 months.
It will be closed in 7 days if no further activity occurs.
Thank you for your contributions.

@stale stale bot added the S-stale label Nov 1, 2021
@bowenwang1996
Copy link
Collaborator

@ailisp is this issue still relevant?

@ailisp
Copy link
Member Author

ailisp commented Nov 2, 2021

Two of them are still relevant, create two PR to address:
#5107
#5108

@matklad
Copy link
Contributor

matklad commented Dec 1, 2021

Was fixed!

@matklad matklad closed this as completed Dec 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-contract-runtime Area: contract compilation and execution, virtual machines, etc C-enhancement Category: An issue proposing an enhancement or a PR with one. T-contract-runtime Team: issues relevant to the contract runtime team
Projects
None yet
Development

No branches or pull requests

3 participants