Review update

stellar · sisuresh · Sep 18, 2024 · Sep 14, 2024 · Aug 13, 2024 · Sep 11, 2024
commit cbbf78a671d222133017489bbc1fb4965511ec65
diff --git a/src/herder/HerderImpl.cpp b/src/herder/HerderImpl.cpp
@@ -60,7 +60,7 @@ constexpr uint32 const CLOSE_TIME_DRIFT_SECONDS_THRESHOLD = 10;
 
 constexpr uint32 const TRANSACTION_QUEUE_TIMEOUT_LEDGERS = 4;
 constexpr uint32 const TRANSACTION_QUEUE_BAN_LEDGERS = 10;
-constexpr uint32 const SOROBAN_TRANSACTION_QUEUE_SIZE_MULTIPLIER = 2;
+constexpr uint32 const TRANSACTION_QUEUE_SIZE_MULTIPLIER = 2;
 
 std::unique_ptr<Herder>
 Herder::create(Application& app)

diff --git a/src/herder/HerderImpl.h b/src/herder/HerderImpl.h
@@ -25,7 +25,7 @@ class Timer;
 
 namespace stellar
 {
-constexpr uint32 const TRANSACTION_QUEUE_SIZE_MULTIPLIER = 2;
+constexpr uint32 const SOROBAN_TRANSACTION_QUEUE_SIZE_MULTIPLIER = 2;
 
 class Application;
 class LedgerManager;

diff --git a/src/main/CommandLine.cpp b/src/main/CommandLine.cpp
@@ -1807,8 +1807,6 @@ runApplyLoad(CommandLineArgs const& args)
 {
     CommandLine::ConfigOption configOption;
 
-    uint32_t numAccounts = 0;
-
     uint64_t ledgerMaxInstructions = 0;
     uint64_t ledgerMaxReadLedgerEntries = 0;
     uint64_t ledgerMaxReadBytes = 0;
@@ -1817,10 +1815,6 @@ runApplyLoad(CommandLineArgs const& args)
     uint64_t ledgerMaxTxCount = 0;
     uint64_t ledgerMaxTransactionsSizeBytes = 0;
 
-    ParserWithValidation numAccountsParser{
-        clara::Arg(numAccounts, "NumAccounts").required(),
-        [&] { return numAccounts > 0 ? "" : "NumAccounts must be > 0"; }};
-
     ParserWithValidation ledgerMaxInstructionsParser{
         clara::Opt(ledgerMaxInstructions,
                    "LedgerMaxInstructions")["--ledger-max-instructions"]
@@ -1889,25 +1883,23 @@ runApplyLoad(CommandLineArgs const& args)
 
     return runWithHelp(
         args,
-        {configurationParser(configOption), numAccountsParser,
-         ledgerMaxInstructionsParser, ledgerMaxReadLedgerEntriesParser,
-         ledgerMaxReadBytesParser, ledgerMaxWriteLedgerEntriesParser,
-         ledgerMaxWriteBytesParser, ledgerMaxTxCountParser,
-         ledgerMaxTransactionsSizeBytesParser},
+        {configurationParser(configOption), ledgerMaxInstructionsParser,
+         ledgerMaxReadLedgerEntriesParser, ledgerMaxReadBytesParser,
+         ledgerMaxWriteLedgerEntriesParser, ledgerMaxWriteBytesParser,
+         ledgerMaxTxCountParser, ledgerMaxTransactionsSizeBytesParser},
         [&] {
             auto config = configOption.getConfig();
             config.RUN_STANDALONE = true;
 
             VirtualClock clock(VirtualClock::REAL_TIME);
-            int result;
             auto appPtr = Application::create(clock, config);
 
             auto& app = *appPtr;
             {
                 auto& lm = app.getLedgerManager();
                 app.start();
 
-                ApplyLoad al(app, numAccounts, ledgerMaxInstructions,
+                ApplyLoad al(app, ledgerMaxInstructions,
                              ledgerMaxReadLedgerEntries, ledgerMaxReadBytes,
                              ledgerMaxWriteLedgerEntries, ledgerMaxWriteBytes,
                              ledgerMaxTxCount, ledgerMaxTransactionsSizeBytes);
@@ -1916,21 +1908,43 @@ runApplyLoad(CommandLineArgs const& args)
                     app.getMetrics().NewTimer({"ledger", "ledger", "close"});
                 ledgerClose.Clear();
 
+                auto& cpuInsRatio = app.getMetrics().NewHistogram(
+                    {"soroban", "host-fn-op",
+                     "invoke-time-fsecs-cpu-insn-ratio"});
+                cpuInsRatio.Clear();
+
+                auto& cpuInsRatioExclVm = app.getMetrics().NewHistogram(
+                    {"soroban", "host-fn-op",
+                     "invoke-time-fsecs-cpu-insn-ratio-excl-vm"});
+                cpuInsRatioExclVm.Clear();
+
                 for (size_t i = 0; i < 20; ++i)
                 {
                     al.benchmark();
                 }
 
                 CLOG_INFO(Perf, "Max ledger close: {} milliseconds",
                           ledgerClose.max());
+                CLOG_INFO(Perf, "Min ledger close: {} milliseconds",
+                          ledgerClose.min());
                 CLOG_INFO(Perf, "Mean ledger close:  {} milliseconds",
                           ledgerClose.mean());
 
+                CLOG_INFO(Perf, "Max CPU ins ratio: {}",
+                          cpuInsRatio.max() / 1000000);
+                CLOG_INFO(Perf, "Mean CPU ins ratio:  {}",
+                          cpuInsRatio.mean() / 1000000);
+
+                CLOG_INFO(Perf, "Max CPU ins ratio excl VM: {}",
+                          cpuInsRatioExclVm.max() / 1000000);
+                CLOG_INFO(Perf, "Mean CPU ins ratio excl VM:  {}",
+                          cpuInsRatioExclVm.mean() / 1000000);
+
                 CLOG_INFO(Perf, "Tx Success Rate: {:f}%",
                           al.successRate() * 100);
             }
 
-            return result;
+            return 0;
         });
 }
 #endif

diff --git a/src/simulation/ApplyLoad.cpp b/src/simulation/ApplyLoad.cpp
@@ -2,6 +2,7 @@
 #include "herder/Herder.h"
 #include "ledger/LedgerManager.h"
 #include "test/TxTests.h"
+#include "transactions/MutableTransactionResult.h"
 #include "transactions/TransactionBridge.h"
 #include "transactions/TransactionUtils.h"
 
@@ -16,14 +17,16 @@
 namespace stellar
 {
 
-ApplyLoad::ApplyLoad(Application& app, uint32_t numAccounts,
-                     uint64_t ledgerMaxInstructions,
+ApplyLoad::ApplyLoad(Application& app, uint64_t ledgerMaxInstructions,
                      uint64_t ledgerMaxReadLedgerEntries,
                      uint64_t ledgerMaxReadBytes,
                      uint64_t ledgerMaxWriteLedgerEntries,
                      uint64_t ledgerMaxWriteBytes, uint64_t ledgerMaxTxCount,
                      uint64_t ledgerMaxTransactionsSizeBytes)
-    : mTxGenerator(app), mApp(app), mNumAccounts(numAccounts)
+    : mTxGenerator(app)
+    , mApp(app)
+    , mNumAccounts(
+          ledgerMaxTxCount * SOROBAN_TRANSACTION_QUEUE_SIZE_MULTIPLIER + 1)
 {
 
     auto rootTestAccount = TestAccount::createRoot(mApp);
@@ -62,9 +65,9 @@ ApplyLoad::ApplyLoad(Application& app, uint32_t numAccounts,
     setupLoadContracts();
 
     // One contract per account
-    releaseAssert(mTxGenerator.GetApplySorobanSuccess().count() ==
-                  numAccounts + 4);
-    releaseAssert(mTxGenerator.GetApplySorobanFailure().count() == 0);
+    releaseAssert(mTxGenerator.getApplySorobanSuccess().count() ==
+                  mNumAccounts + 4);
+    releaseAssert(mTxGenerator.getApplySorobanFailure().count() == 0);
 }
 
 void
@@ -181,7 +184,7 @@ ApplyLoad::setupLoadContracts()
 
     closeLedger({uploadTx.second});
 
-    for (auto kvp : mTxGenerator.getAccounts())
+    for (auto const& kvp : mTxGenerator.getAccounts())
     {
         auto salt = sha256("Load contract " + std::to_string(kvp.first));
 
@@ -207,8 +210,8 @@ ApplyLoad::benchmark()
     auto& lm = mApp.getLedgerManager();
     std::vector<TransactionFrameBasePtr> txs;
 
-    auto resources = multiplyByDouble(lm.maxLedgerResources(true),
-                                      TRANSACTION_QUEUE_SIZE_MULTIPLIER);
+    auto resources = multiplyByDouble(
+        lm.maxLedgerResources(true), SOROBAN_TRANSACTION_QUEUE_SIZE_MULTIPLIER);
 
     auto const& accounts = mTxGenerator.getAccounts();
     std::vector<uint64_t> shuffledAccounts(accounts.size());
@@ -228,24 +231,34 @@ ApplyLoad::benchmark()
             lm.getLastClosedLedgerNum() + 1, it->first, instance,
             rust_bridge::get_write_bytes().data.size() + 160, std::nullopt);
 
+        {
+            LedgerTxn ltx(mApp.getLedgerTxnRoot());
+            auto res = tx.second->checkValid(mApp, ltx, 0, 0, UINT64_MAX);
+            releaseAssert((res && res->isSuccess()));
+        }
+
         if (!anyGreater(tx.second->getResources(false), resources))
         {
             resources -= tx.second->getResources(false);
         }
         else
         {
-            for (size_t i = static_cast<size_t>(Resource::Type::OPERATIONS);
-                 i <= static_cast<size_t>(Resource::Type::WRITE_LEDGER_ENTRIES);
-                 ++i)
+            bool limitHit = false;
+            for (size_t i = 0; i < resources.size(); ++i)
             {
                 auto type = static_cast<Resource::Type>(i);
                 if (tx.second->getResources(false).getVal(type) >
                     resources.getVal(type))
                 {
                     CLOG_INFO(Perf, "Ledger {} limit hit during tx generation",
                               Resource::getStringFromType(type));
+                    limitHit = true;
                 }
             }
+
+            // If this assert fails, it most likely means that we ran out of
+            // accounts, which should not happen.
+            releaseAssert(limitHit);
             break;
         }
 
@@ -258,9 +271,9 @@ ApplyLoad::benchmark()
 double
 ApplyLoad::successRate()
 {
-    return mTxGenerator.GetApplySorobanSuccess().count() * 1.0 /
-           (mTxGenerator.GetApplySorobanSuccess().count() +
-            mTxGenerator.GetApplySorobanFailure().count());
+    return mTxGenerator.getApplySorobanSuccess().count() * 1.0 /
+           (mTxGenerator.getApplySorobanSuccess().count() +
+            mTxGenerator.getApplySorobanFailure().count());
 }
 
 }
diff --git a/src/simulation/ApplyLoad.h b/src/simulation/ApplyLoad.h
@@ -1,3 +1,8 @@
+// Copyright 2024 Stellar Development Foundation and contributors. Licensed
+// under the Apache License, Version 2.0. See the COPYING file at the root
+// of this distribution or at http://www.apache.org/licenses/LICENSE-2.0
+
+#pragma once
 #include "main/Application.h"
 #include "simulation/TxGenerator.h"
 #include "test/TestAccount.h"
@@ -7,8 +12,7 @@ namespace stellar
 class ApplyLoad
 {
   public:
-    ApplyLoad(Application& app, uint32_t numAccounts,
-              uint64_t ledgerMaxInstructions,
+    ApplyLoad(Application& app, uint64_t ledgerMaxInstructions,
               uint64_t ledgerMaxReadLedgerEntries, uint64_t ledgerMaxReadBytes,
               uint64_t ledgerMaxWriteLedgerEntries,
               uint64_t ledgerMaxWriteBytes, uint64_t ledgerMaxTxCount,

diff --git a/src/simulation/LoadGenerator.cpp b/src/simulation/LoadGenerator.cpp
@@ -382,9 +382,9 @@ LoadGenerator::start(GeneratedLoadConfig& cfg)
     releaseAssert(mPreLoadgenApplySorobanSuccess == 0);
     releaseAssert(mPreLoadgenApplySorobanFailure == 0);
     mPreLoadgenApplySorobanSuccess =
-        mTxGenerator.GetApplySorobanSuccess().count();
+        mTxGenerator.getApplySorobanSuccess().count();
     mPreLoadgenApplySorobanFailure =
-        mTxGenerator.GetApplySorobanFailure().count();
+        mTxGenerator.getApplySorobanFailure().count();
 
     mStarted = true;
 }
@@ -719,50 +719,14 @@ LoadGenerator::generateLoad(GeneratedLoadConfig cfg)
                     if (sorobanCfg.nWasms != 0)
                     {
                         --sorobanCfg.nWasms;
-
-                        auto wasm = cfg.modeSetsUpInvoke()
-                                        ? rust_bridge::get_test_wasm_loadgen()
-                                        : rust_bridge::get_write_bytes();
-                        xdr::opaque_vec<> wasmBytes;
-                        wasmBytes.assign(wasm.data.begin(), wasm.data.end());
-
-                        LedgerKey contractCodeLedgerKey;
-                        contractCodeLedgerKey.type(CONTRACT_CODE);
-                        contractCodeLedgerKey.contractCode().hash =
-                            sha256(wasmBytes);
-
-                        releaseAssert(!mCodeKey);
-                        mCodeKey = contractCodeLedgerKey;
-
-                        // Wasm blob + approximate overhead for contract
-                        // instance and ContractCode LE overhead
-                        mContactOverheadBytes = wasmBytes.size() + 160;
-
-                        return mTxGenerator.createUploadWasmTransaction(
-                            ledgerNum, sourceAccountId, wasmBytes, *mCodeKey,
-                            cfg.maxGeneratedFeeRate);
+                        return createUploadWasmTransaction(cfg, ledgerNum,
+                                                           sourceAccountId);
                     }
                     else
                     {
                         --sorobanCfg.nInstances;
-
-                        auto salt =
-                            sha256("upgrade" +
-                                   std::to_string(
-                                       ++mNumCreateContractTransactionCalls));
-
-                        auto txPair = mTxGenerator.createContractTransaction(
-                            ledgerNum, sourceAccountId, *mCodeKey,
-                            mContactOverheadBytes, salt,
-                            cfg.maxGeneratedFeeRate);
-
-                        auto const& instanceLk =
-                            txPair.second->sorobanResources()
-                                .footprint.readWrite.back();
-
-                        mContractInstanceKeys.emplace(instanceLk);
-
-                        return txPair;
+                        return createInstanceTransaction(cfg, ledgerNum,
+                                                         sourceAccountId);
                     }
                 };
                 break;
@@ -1094,6 +1058,52 @@ LoadGenerator::createMixedClassicSorobanTransaction(
     }
 }
 
+std::pair<TxGenerator::TestAccountPtr, TransactionTestFramePtr>
+LoadGenerator::createUploadWasmTransaction(GeneratedLoadConfig const& cfg,
+                                           uint32_t ledgerNum,
+                                           uint64_t sourceAccountId)
+{
+    auto wasm = cfg.modeSetsUpInvoke() ? rust_bridge::get_test_wasm_loadgen()
+                                       : rust_bridge::get_write_bytes();
+    xdr::opaque_vec<> wasmBytes;
+    wasmBytes.assign(wasm.data.begin(), wasm.data.end());
+
+    LedgerKey contractCodeLedgerKey;
+    contractCodeLedgerKey.type(CONTRACT_CODE);
+    contractCodeLedgerKey.contractCode().hash = sha256(wasmBytes);
+
+    releaseAssert(!mCodeKey);
+    mCodeKey = contractCodeLedgerKey;
+
+    // Wasm blob + approximate overhead for contract
+    // instance and ContractCode LE overhead
+    mContactOverheadBytes = wasmBytes.size() + 160;
+
+    return mTxGenerator.createUploadWasmTransaction(ledgerNum, sourceAccountId,
+                                                    wasmBytes, *mCodeKey,
+                                                    cfg.maxGeneratedFeeRate);
+}
+
+std::pair<TxGenerator::TestAccountPtr, TransactionTestFramePtr>
+LoadGenerator::createInstanceTransaction(GeneratedLoadConfig const& cfg,
+                                         uint32_t ledgerNum,
+                                         uint64_t sourceAccountId)
+{
+    auto salt = sha256("upgrade" +
+                       std::to_string(++mNumCreateContractTransactionCalls));
+
+    auto txPair = mTxGenerator.createContractTransaction(
+        ledgerNum, sourceAccountId, *mCodeKey, mContactOverheadBytes, salt,
+        cfg.maxGeneratedFeeRate);
+
+    auto const& instanceLk =
+        txPair.second->sorobanResources().footprint.readWrite.back();
+
+    mContractInstanceKeys.emplace(instanceLk);
+
+    return txPair;
+}
+
 void
 LoadGenerator::maybeHandleFailedTx(TransactionFrameBaseConstPtr tx,
                                    TxGenerator::TestAccountPtr sourceAccount,
@@ -1206,8 +1216,8 @@ LoadGenerator::checkMinimumSorobanSuccess(GeneratedLoadConfig const& cfg)
         return true;
     }
 
-    int64_t nTxns = mTxGenerator.GetApplySorobanSuccess().count() +
-                    mTxGenerator.GetApplySorobanFailure().count() -
+    int64_t nTxns = mTxGenerator.getApplySorobanSuccess().count() +
+                    mTxGenerator.getApplySorobanFailure().count() -
                     mPreLoadgenApplySorobanSuccess -
                     mPreLoadgenApplySorobanFailure;
 
@@ -1217,7 +1227,7 @@ LoadGenerator::checkMinimumSorobanSuccess(GeneratedLoadConfig const& cfg)
         return true;
     }
 
-    int64_t nSuccessful = mTxGenerator.GetApplySorobanSuccess().count() -
+    int64_t nSuccessful = mTxGenerator.getApplySorobanSuccess().count() -
                           mPreLoadgenApplySorobanSuccess;
     return (nSuccessful * 100) / nTxns >= cfg.getMinSorobanPercentSuccess();
 }