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.

Sign up for GitHub

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

Jump to bottom

Add VM concurrency support #2059

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from
Draft

Add VM concurrency support #2059

wants to merge 6 commits into from
+77 −29

Conversation

AnkushinDaniil
Copy link
Contributor

Fixes #2016

This PR adds a new flag, VMConcurrencyMode and changes cairoVMExecute.

@AnkushinDaniil
Copy link
Contributor Author

Tested by running ./build/juno --http --vm-concurrency-mode and requesting

{
	"jsonrpc":"2.0",
	"method":"starknet_traceTransaction",
	"params":["0x5634f2847140263ba59480ad4781dacc9991d0365145489b27a198ebed2f969"],
	"id":1
}

Response body

{
  "jsonrpc": "2.0",
  "result": {
    "type": "INVOKE",
    "execute_invocation": {
      "contract_address": "0x31c9cdb9b00cb35cf31c05855c0ec3ecf6f7952a1ce6e3c53c3455fcd75a280",
      "calldata": [
        "0x20cfa74ee3564b4cd5435cdace0f9c4d43b939620e4a0bb5076105df0a626c6",
        "0x5aee31408163292105d875070f98cb48275b8c87e80380b78d30647e05854d5"
      ],
      "caller_address": "0x0",
      "result": [],
      "calls": [
        {
          "contract_address": "0x20cfa74ee3564b4cd5435cdace0f9c4d43b939620e4a0bb5076105df0a626c6",
          "calldata": [
            "0x5aee31408163292105d875070f98cb48275b8c87e80380b78d30647e05854d5",
            "0x7e5"
          ],
          "caller_address": "0x31c9cdb9b00cb35cf31c05855c0ec3ecf6f7952a1ce6e3c53c3455fcd75a280",
          "result": [],
          "calls": [],
          "events": [],
          "messages": [],
          "execution_resources": { "steps": 25 }
        },
        {
          "contract_address": "0x20cfa74ee3564b4cd5435cdace0f9c4d43b939620e4a0bb5076105df0a626c6",
          "calldata": [
            "0x5aee31408163292105d875070f98cb48275b8c87e80380b78d30647e05854d5"
          ],
          "caller_address": "0x31c9cdb9b00cb35cf31c05855c0ec3ecf6f7952a1ce6e3c53c3455fcd75a280",
          "result": ["0x7e5"],
          "calls": [],
          "events": [],
          "messages": [],
          "execution_resources": { "steps": 31 }
        }
      ],
      "events": [],
      "messages": [],
      "execution_resources": { "steps": 178 }
    }
  },
  "id": 1
}

@AnkushinDaniil AnkushinDaniil self-assigned this Aug 16, 2024
@AnkushinDaniil AnkushinDaniil added VM go Pull requests that update Go code rust Pull requests that update Rust code labels Aug 16, 2024
@codecov Codecov
Copy link

codecov bot commented Aug 16, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 78.47%. Comparing base (67430de) to head (a053ddc).
Report is 74 commits behind head on main.

Additional details and impacted files 
@@           Coverage Diff           @@
##             main    #2059   +/-   ##
=======================================
  Coverage   78.47%   78.47%           
=======================================
  Files         100      100           
  Lines        9234     9235    +1     
=======================================
+ Hits         7246     7247    +1     
  Misses       1350     1350           
  Partials      638      638

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@AnkushinDaniil AnkushinDaniil marked this pull request as draft August 19, 2024 14:46
@AnkushinDaniil AnkushinDaniil force-pushed the daniil/add-vm-concurrency-support branch 3 times, most recently from 530d149 to f7f7a60 Compare August 22, 2024 11:24
@AnkushinDaniil
Copy link
Contributor Author

The current problem is this bug. It happened in sequential mode.
My hypotheses were:

  1. Transaction preparation loop:
    In the updated code, transactions are first prepared and stored in a Vec<Transaction> before being executed. This is different from the old version, where each transaction was processed immediately after preparation. This change in flow can cause the error.
  2. State management:
    In the old code, the CachedState is modified transactionally and committed after each transaction. In the new code, transactions are prepared, executed in a batch, and then the state is committed. Delaying the commit of state changes until all transactions are executed might lead to issues.
  3. The charge_fee and validate are hardcoded as true in the transaction executor, this might cause the error. I changed them both to false, it doesn't help.

@AnkushinDaniil AnkushinDaniil force-pushed the daniil/add-vm-concurrency-support branch from f7f7a60 to 2f33d5a Compare August 23, 2024 11:08
@AnkushinDaniil
Copy link
Contributor Author

To make it work in my branch:

  1. in vm/rust run make clean
  2. here and here validate: false
  3. in vm/rust run make all
  4. make juno
  5. ./build/juno --http --network sepolia and ran a test query
  6. ./build/juno --http --network sepolia --vm-concurrency-mode and ran a test query
    test query:
{
    "id": 1,
    "jsonrpc": "2.0",
    "method": "starknet_estimateFee",
    "params": {
        "request": [
            {
            "type": "INVOKE",
            "sender_address": "0xf9e998b2853e6d01f3ae3c598c754c1b9a7bd398fec7657de022f3b778679",
            "calldata": [
                "0x1",
                "0x41a78e741e5af2fec34b695679bc6891742439f7afb8484ecd7766661ad02bf",
                "0x1987cbd17808b9a23693d4de7e246a443cfe37e6e7fbaeabd7d7e6532b07c3d",
                "0x4",
                "0x16342ade8a7cc8296920731bc34b5a6530f5ee1dc1bfd3cc83cb3f519d6530a",
                "0x77295b07f4b1d4e4fdb18b26faf95d5bdb0087da065b7b0b49d0470d9cbf852",
                "0x1",
                "0x0"
            ],
            "version": "0x100000000000000000000000000000001",
            "signature": [],
            "nonce": "0x2a91",
            "max_fee": "0x0"
            }
        ],
        "block_id": "pending",
        "simulation_flags": [
            "SKIP_VALIDATE"
        ]
    }
}

@AnkushinDaniil
Copy link
Contributor Author

Pathfinder team encountered the same problem

@rianhughes
Copy link
Contributor

rianhughes commented Aug 26, 2024 •
edited
Loading

It would be good to add a test for the concurrency mode if possible.

Also curious what the performance of simulating a block in concurrency mode is as well.

@rianhughes rianhughes self-requested a review August 26, 2024 09:14
@AnkushinDaniil AnkushinDaniil force-pushed the daniil/add-vm-concurrency-support branch from 2f33d5a to 792fc56 Compare August 27, 2024 09:56
@AnkushinDaniil
Copy link
Contributor Author

I tried to measure performance by running multiple transactions and found a new bug.
The error is Transaction with no fee transfer info must have zero fee.
This happens here only in concurrency mode. According to blockifier, in both modes L1HandlerTransaction must have fee_transfer_call_info: None , i.e. problem in tx_execution_info.receipt.fee, it's not equal to 0.

@AnkushinDaniil AnkushinDaniil force-pushed the daniil/add-vm-concurrency-support branch 3 times, most recently from 913f964 to dd5ec08 Compare September 6, 2024 15:06
@AnkushinDaniil
Copy link
Contributor Author

AnkushinDaniil commented Sep 6, 2024 •
edited
Loading

Also curious what the performance of simulating a block in concurrency mode is as well.

I roughly measured the performance difference. I used a slice of different txs from sepolia block №84662 with length 89.
I measured the time of cairoVMExecute execution.
Results (average):

  1. Original version: 100%
  2. Concurrency support: sequential mode: ~99,59%
  3. Concurrency support: concurrent mode: ~115,76%

I'm trying to get a profile of each version, but I get a weird result
I get ExternalCode only in concurrent mode

image

In addition, here are the original measurements:

image

Number of concurrent measurements more because I have a timeout = 60 seconds

@AnkushinDaniil
Add concurrency support
dcacb48 
This commit changes cairoVMExecute. It doesn't use the `charge_fee` and `validate` variables. It converts the `TxnAndQueryBit` objects to the `Transaction` objects expected by the TransactionExecutor, uses the `execute_txs` method of the `TransactionExecutor` to execute all transactions (concurrently or not). After execution, it handles the results, including errors, rolled back transactions, and trace logging, according to the original logic.
@AnkushinDaniil
Add num_cpus
e60691d
@AnkushinDaniil
Update TransactionExecutorConfig concurrency settings
b2af037
@AnkushinDaniil
Update blockifier dependency to version 0.8.0-rc.2 with concurrency f…
a053ddc 
…eature
@AnkushinDaniil AnkushinDaniil force-pushed the daniil/add-vm-concurrency-support branch from dd5ec08 to a053ddc Compare September 24, 2024 10:09
@github-actions GitHub Actions
Copy link
Contributor

This pull request is stale because it has been open 35 days with no activity. Remove stale label or comment or this will be closed in 14 days.

@github-actions github-actions bot added Stale and removed Stale labels Nov 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@AnkushinDaniil AnkushinDaniil

Labels
go Pull requests that update Go code rust Pull requests that update Rust code VM
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add vm concurrency support
3 participants
@AnkushinDaniil @rianhughes @wojciechos