Use `DependentCost` for `aloc` opcode #1934

xgreenx · 2024-06-04T10:19:58Z

PR upgrades the fuel-vm to 0.52.0. A new VM changes aloc opcode to be DependentCost. It requires updated benchmarks.

The PR also updates default gas costs according to a new Vultr instance we use to do benchmarks. It works faster than the previous one, so all opcodes became cheaper.

The vm_initialziation became much cheaper and now has reasonable values. The bloc opcode became expensive and depended on a number of allocated bytes.

Checklist

Breaking changes are clearly marked as such in the PR description and changelog

Before requesting review

I have reviewed the code myself

After merging, notify other teams

Both SDKs

# Conflicts: # CHANGELOG.md # Cargo.lock

…o/reuse-vm-memory

# Conflicts: # Cargo.lock # benches/benches/vm_initialization.rs

Voxelot · 2024-06-06T00:56:56Z

crates/client/src/client/schema/chain.rs

@@ -324,10 +324,9 @@ impl TryFrom<GasCosts> for fuel_core_types::fuel_tx::GasCosts {
    fn try_from(value: GasCosts) -> Result<Self, Self::Error> {
        match value.version {
            GasCostsVersion::V1 => {


Why is GasCostsVersion still V1 but GasCostsValues is now V2?

GraphQL version doesn't need to match the Rust version of the type. It was done to maintain backward compatibility. Because SDKs will throw an error if the version is unknown.

Will it get confusing if the versions start diverging a lot over time?

When it will diverging a lot, we just can jump straight to the highest=) I was done for backward compatibility until SDKs will handle it gracefully.

Voxelot · 2024-06-06T00:58:17Z

...schema/snapshots/fuel_core_client__client__schema__chain__tests__chain_gql_query_output.snap

@@ -82,7 +82,6 @@ query {
        version
        add
        addi
-        aloc


could this break older SDK versions loading the gas costs for predicate estimation?

Voxelot · 2024-06-06T01:09:37Z

crates/fuel-core/src/schema/chain.rs

@@ -338,7 +338,7 @@ impl GasCosts {
    }

    async fn aloc(&self) -> U64 {


disregard my earlier comment, it looks like this will retain backwards compatibility

Yep, the only problem is that if Rust SDK is old and connected to fuel-core 0.27.0, the predicate estimation can be incorrect if it uses an alco opcode. But it should be addressed on Rust SDK side: FuelLabs/fuels-rs#1409

Dentosal · 2024-06-06T03:04:29Z

benches/benches/vm_initialization.rs

+                #[allow(clippy::unit_arg)]
+                black_box(


Does this black_box actually do anything?

Removing black_box improves speed

MitchTurner · 2024-06-06T17:48:07Z

Cargo.toml

@@ -81,7 +81,7 @@ fuel-core-wasm-executor = { version = "0.27.0", path = "./crates/services/upgrad
 fuel-core-xtask = { version = "0.0.0", path = "./xtask" }

 # Fuel dependencies
-fuel-vm-private = { version = "0.51.0", package = "fuel-vm", default-features = false }
+fuel-vm-private = { version = "0.52.0", package = "fuel-vm", default-features = false }


The changelog says your updating to 0.51.0, not 0.52.0.

[#1888](https://github.com/FuelLabs/fuel-core/pull/1888): Upgraded `fuel-vm` to `0.51.0`. See [release](https://github.com/FuelLabs/fuel-vm/releases/tag/v0.51.0) for more information.

The changelog related for 0.51.0 is from another PR, I just grouped everything together=)

seems like we are being inconsistent about when we note these things, but I don't know that it matters

MitchTurner · 2024-06-06T17:57:21Z

crates/client/src/client/schema/chain.rs

@@ -324,10 +324,9 @@ impl TryFrom<GasCosts> for fuel_core_types::fuel_tx::GasCosts {
    fn try_from(value: GasCosts) -> Result<Self, Self::Error> {
        match value.version {
            GasCostsVersion::V1 => {


Will it get confusing if the versions start diverging a lot over time?

MitchTurner · 2024-06-06T22:21:53Z

Cargo.toml

@@ -81,7 +81,7 @@ fuel-core-wasm-executor = { version = "0.27.0", path = "./crates/services/upgrad
 fuel-core-xtask = { version = "0.0.0", path = "./xtask" }

 # Fuel dependencies
-fuel-vm-private = { version = "0.51.0", package = "fuel-vm", default-features = false }
+fuel-vm-private = { version = "0.52.0", package = "fuel-vm", default-features = false }


seems like we are being inconsistent about when we note these things, but I don't know that it matters

@xgreenx

## Version v0.28.0 ### Changed - [#1934](#1934): Updated benchmark for the `aloc` opcode to be `DependentCost`. Updated `vm_initialization` benchmark to exclude growing of memory(It is handled by VM reuse). - [#1916](#1916): Speed up synchronisation of the blocks for the `fuel-core-sync` service. - [#1888](#1888): optimization: Reuse VM memory across executions. #### Breaking - [#1934](#1934): Changed `GasCosts` endpoint to return `DependentCost` for the `aloc` opcode via `alocDependentCost`. - [#1934](#1934): Updated default gas costs for the local testnet configuration. All opcodes became cheaper. - [#1924](#1924): `dry_run_opt` has new `gas_price: Option<u64>` argument - [#1888](#1888): Upgraded `fuel-vm` to `0.51.0`. See [release](https://github.com/FuelLabs/fuel-vm/releases/tag/v0.51.0) for more information. ### Added - [#1939](#1939): Added API functions to open a RocksDB in different modes. - [#1929](#1929): Added support of customization of the state transition version in the `ChainConfig`. ### Removed - [#1913](#1913): Removed dead code from the project. ### Fixed - [#1921](#1921): Fixed unstable `gossipsub_broadcast_tx_with_accept` test. - [#1915](#1915): Fixed reconnection issue in the dev cluster with AWS cluster. - [#1914](#1914): Fixed halting of the node during synchronization in PoA service. ## What's Changed * Removed dead code by @xgreenx in #1913 * Added backward and forward compatibility integration tests for forkless upgrades by @xgreenx in #1895 * Fixed halting of the node in rare conditions by @xgreenx in #1914 * Weekly `cargo update` by @github-actions in #1928 * Fixed logging of the WASM executor by @xgreenx in #1930 * Added support of customization of the state transition version in the `ChainConfig` by @xgreenx in #1929 * Document wasm toolchain installation, add rust-toolchain.toml by @Dentosal in #1932 * Add optional `gas_price` argument to `dry_run_opt` by @hal3e in #1924 * Reuse VM memory across executions by @Dentosal in #1888 * Fixed reconnection issue in the dev cluster with AWS cluster by @xgreenx in #1915 * Speeds up synchronisation of the blocks for the `fuel-core-sync` service by @xgreenx in #1916 * Fixed unstable `gossipsub_broadcast_tx_with_accept` test by @xgreenx in #1921 * Added API functions to open a RocksDB in different modes by @xgreenx in #1939 * Use `DependentCost` for `aloc` opcode by @xgreenx in #1934 ## New Contributors * @hal3e made their first contribution in #1924 **Full Changelog**: v0.27.0...v0.28.0

@xgreenx

## Version v0.28.0 ### Changed - [#1934](FuelLabs/fuel-core#1934): Updated benchmark for the `aloc` opcode to be `DependentCost`. Updated `vm_initialization` benchmark to exclude growing of memory(It is handled by VM reuse). - [#1916](FuelLabs/fuel-core#1916): Speed up synchronisation of the blocks for the `fuel-core-sync` service. - [#1888](FuelLabs/fuel-core#1888): optimization: Reuse VM memory across executions. #### Breaking - [#1934](FuelLabs/fuel-core#1934): Changed `GasCosts` endpoint to return `DependentCost` for the `aloc` opcode via `alocDependentCost`. - [#1934](FuelLabs/fuel-core#1934): Updated default gas costs for the local testnet configuration. All opcodes became cheaper. - [#1924](FuelLabs/fuel-core#1924): `dry_run_opt` has new `gas_price: Option<u64>` argument - [#1888](FuelLabs/fuel-core#1888): Upgraded `fuel-vm` to `0.51.0`. See [release](https://github.com/FuelLabs/fuel-vm/releases/tag/v0.51.0) for more information. ### Added - [#1939](FuelLabs/fuel-core#1939): Added API functions to open a RocksDB in different modes. - [#1929](FuelLabs/fuel-core#1929): Added support of customization of the state transition version in the `ChainConfig`. ### Removed - [#1913](FuelLabs/fuel-core#1913): Removed dead code from the project. ### Fixed - [#1921](FuelLabs/fuel-core#1921): Fixed unstable `gossipsub_broadcast_tx_with_accept` test. - [#1915](FuelLabs/fuel-core#1915): Fixed reconnection issue in the dev cluster with AWS cluster. - [#1914](FuelLabs/fuel-core#1914): Fixed halting of the node during synchronization in PoA service. ## What's Changed * Removed dead code by @xgreenx in FuelLabs/fuel-core#1913 * Added backward and forward compatibility integration tests for forkless upgrades by @xgreenx in FuelLabs/fuel-core#1895 * Fixed halting of the node in rare conditions by @xgreenx in FuelLabs/fuel-core#1914 * Weekly `cargo update` by @github-actions in FuelLabs/fuel-core#1928 * Fixed logging of the WASM executor by @xgreenx in FuelLabs/fuel-core#1930 * Added support of customization of the state transition version in the `ChainConfig` by @xgreenx in FuelLabs/fuel-core#1929 * Document wasm toolchain installation, add rust-toolchain.toml by @Dentosal in FuelLabs/fuel-core#1932 * Add optional `gas_price` argument to `dry_run_opt` by @hal3e in FuelLabs/fuel-core#1924 * Reuse VM memory across executions by @Dentosal in FuelLabs/fuel-core#1888 * Fixed reconnection issue in the dev cluster with AWS cluster by @xgreenx in FuelLabs/fuel-core#1915 * Speeds up synchronisation of the blocks for the `fuel-core-sync` service by @xgreenx in FuelLabs/fuel-core#1916 * Fixed unstable `gossipsub_broadcast_tx_with_accept` test by @xgreenx in FuelLabs/fuel-core#1921 * Added API functions to open a RocksDB in different modes by @xgreenx in FuelLabs/fuel-core#1939 * Use `DependentCost` for `aloc` opcode by @xgreenx in FuelLabs/fuel-core#1934 ## New Contributors * @hal3e made their first contribution in FuelLabs/fuel-core#1924 **Full Changelog**: FuelLabs/fuel-core@v0.27.0...v0.28.0

Dentosal and others added 30 commits May 8, 2024 04:37

Reuse VM memory across executions

81dc78a

Add changelog entry

42105ce

Merge branch 'master' into dento/reuse-vm-memory

e14cf0b

Switch to a unified pool instead of per-thread one

5caec42

Move VmPool to fuel-vm, use it for predicates as well

6340785

Cleanup

22fa926

Cleanup

8c276e9

Fix benches

0b11719

Add changes from the latest fuel-vm side update

4c6f29b

Match fuel-vm changes

22f3ba4

Merge branch 'master' into dento/reuse-vm-memory

4f9340c

cargo sort

57d2049

Merge branch 'refs/heads/master' into dento/reuse-vm-memory

0e41a28

# Conflicts: # CHANGELOG.md # Cargo.lock

Use fuel-vm 0.51.0

a762483

Merge branch 'master' into dento/reuse-vm-memory

131a91e

Make clippy happy

98c7495

Fix CI

395077b

Fix CI

36fa8be

Fix CI

3baea26

Fixed VM initialization benchmark

b13578f

Merge branch 'master' into dento/reuse-vm-memory

eb65aba

Use release 0.51

5196bab

Merge remote-tracking branch 'origin/dento/reuse-vm-memory' into dent…

2e4bc5b

…o/reuse-vm-memory

Make clippy happy

c2034bb

Fixed becnhmark for aloc opcode

ec2a65b

Fixed becnhmark for aloc opcode

fc15915

Not include cloning of memory into the benchamrk results

6000f11

Return back old benchmark

56915e7

Make aloc depdedent

564fec0

Remove warnings

a719693

xgreenx self-assigned this Jun 4, 2024

xgreenx changed the title ~~Feature/benchmarks alloc~~ Use DependentCost for aloc opcode Jun 4, 2024

xgreenx mentioned this pull request Jun 4, 2024

Use DependentCost for aloc opcode FuelLabs/fuel-vm#747

Merged

3 tasks

Use V2 gas costs

de2ec13

Base automatically changed from dento/reuse-vm-memory to master June 4, 2024 18:45

xgreenx added 4 commits June 5, 2024 12:34

Merge branch 'refs/heads/master' into feature/benchmarks-alloc

7ac5fa5

# Conflicts: # Cargo.lock # benches/benches/vm_initialization.rs

Updated gas cost to match the current version of the executor

62e6f24

Merge branch 'refs/heads/master' into feature/benchmarks-alloc

ae90adf

Updated CHANGELOG.md

cbd73d9

xgreenx requested a review from a team June 5, 2024 12:07

xgreenx marked this pull request as ready for review June 5, 2024 12:07

Fixed test

13fe891

xgreenx requested review from MitchTurner and Dentosal June 5, 2024 19:21

Voxelot reviewed Jun 6, 2024

View reviewed changes

Dentosal reviewed Jun 6, 2024

View reviewed changes

xgreenx requested review from Voxelot and Dentosal June 6, 2024 08:33

MitchTurner reviewed Jun 6, 2024

View reviewed changes

xgreenx requested a review from MitchTurner June 6, 2024 22:17

Merge branch 'master' into feature/benchmarks-alloc

703033a

MitchTurner approved these changes Jun 6, 2024

View reviewed changes

xgreenx enabled auto-merge (squash) June 6, 2024 22:29

xgreenx merged commit 4986d4d into master Jun 6, 2024
30 checks passed

xgreenx deleted the feature/benchmarks-alloc branch June 6, 2024 22:36

xgreenx mentioned this pull request Jun 6, 2024

Release v0.28.0 #1945

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use `DependentCost` for `aloc` opcode #1934

Use `DependentCost` for `aloc` opcode #1934

xgreenx commented Jun 4, 2024 •

edited

Loading

Voxelot Jun 6, 2024 •

edited

Loading

xgreenx Jun 6, 2024

MitchTurner Jun 6, 2024

xgreenx Jun 6, 2024

Voxelot Jun 6, 2024

Voxelot Jun 6, 2024

xgreenx Jun 6, 2024

Dentosal Jun 6, 2024

xgreenx Jun 6, 2024

MitchTurner Jun 6, 2024

MitchTurner Jun 6, 2024

xgreenx Jun 6, 2024

MitchTurner Jun 6, 2024

MitchTurner Jun 6, 2024

MitchTurner Jun 6, 2024

@@ @@ -82,7 +82,6 @@ query { @@
                       version
                       add
                       addi
-                      aloc

		@@ -338,7 +338,7 @@ impl GasCosts {
		}

		async fn aloc(&self) -> U64 {

Use DependentCost for aloc opcode #1934

Use DependentCost for aloc opcode #1934

Conversation

xgreenx commented Jun 4, 2024 • edited Loading

Checklist

Before requesting review

After merging, notify other teams

Voxelot Jun 6, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Use `DependentCost` for `aloc` opcode #1934

Use `DependentCost` for `aloc` opcode #1934

xgreenx commented Jun 4, 2024 •

edited

Loading

Voxelot Jun 6, 2024 •

edited

Loading