Skip to content

Commit

Permalink
Undo unnecessary metrics change
Browse files Browse the repository at this point in the history
  • Loading branch information
@sisuresh
sisuresh committed Jul 3, 2024
1 parent 285e3d3 commit 8d4c7df
Show file tree
Hide file tree
Showing 13 changed files with 67 additions and 221 deletions.
12 changes: 6 additions & 6 deletions src/ledger/LedgerManagerImpl.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1564,6 +1564,7 @@ LedgerManagerImpl::applyThread(ThreadEntryMap& entryMap, Thread const& thread,
SorobanNetworkConfig const& sorobanConfig,
CxxLedgerInfo const& ledgerInfo,
Hash const& sorobanBasePrngSeed,
SorobanMetrics& sorobanMetrics,
uint32_t ledgerSeq, uint32_t ledgerVersion)
{
// TTL extensions can't be observable between transactions, so we track them
Expand All @@ -1576,7 +1577,8 @@ LedgerManagerImpl::applyThread(ThreadEntryMap& entryMap, Thread const& thread,
releaseAssertOrThrow(txBundle.resPayload);
auto res = txBundle.tx->parallelApply(
entryMap, config, sorobanConfig, ledgerInfo, txBundle.resPayload,
sorobanBasePrngSeed, txBundle.meta, ledgerSeq, ledgerVersion);
sorobanMetrics, sorobanBasePrngSeed, txBundle.meta, ledgerSeq,
ledgerVersion);

if (res.mSuccess)
{
Expand Down Expand Up @@ -1630,7 +1632,6 @@ LedgerManagerImpl::applyThread(ThreadEntryMap& entryMap, Thread const& thread,
releaseAssertOrThrow(txBundle.resPayload->getResultCode() ==
txFAILED);
}
txBundle.mOpMetrics = res.mOpMetrics;
txBundle.mDelta = res.mDelta;
}

Expand Down Expand Up @@ -1711,6 +1712,7 @@ LedgerManagerImpl::applySorobanStage(Application& app, AbstractLedgerTxn& ltx,
auto const& config = app.getConfig();
auto const& sorobanConfig =
app.getLedgerManager().getSorobanNetworkConfig();
auto& sorobanMetrics = app.getLedgerManager().getSorobanMetrics();

uint32_t ledgerSeq = ltx.loadHeader().current().ledgerSeq;
uint32_t ledgerVersion = ltx.loadHeader().current().ledgerVersion;
Expand All @@ -1737,7 +1739,8 @@ LedgerManagerImpl::applySorobanStage(Application& app, AbstractLedgerTxn& ltx,
threads.push_back(std::thread(
&LedgerManagerImpl::applyThread, this, std::ref(entryMapByThread),
std::ref(thread), config, sorobanConfig, std::ref(ledgerInfo),
sorobanBasePrngSeed, ledgerSeq, ledgerVersion));
sorobanBasePrngSeed, std::ref(sorobanMetrics), ledgerSeq,
ledgerVersion));
}

// TODO: This will change when we use a thread pool.
Expand Down Expand Up @@ -1780,9 +1783,6 @@ LedgerManagerImpl::applySorobanStage(Application& app, AbstractLedgerTxn& ltx,
txBundle.tx->processPostApply(mApp, ltxInner, txBundle.meta,
txBundle.resPayload);

releaseAssertOrThrow(txBundle.mOpMetrics);
txBundle.mOpMetrics->updateSorobanMetrics(mSorobanMetrics);

// We only increase the internal-error metric count if the
// ledger is a newer version.
if (txBundle.resPayload->getResultCode() == txINTERNAL_ERROR &&
Expand Down
3 changes: 2 additions & 1 deletion src/ledger/LedgerManagerImpl.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,8 @@ class LedgerManagerImpl : public LedgerManager
Config const& config,
SorobanNetworkConfig const& sorobanConfig,
CxxLedgerInfo const& ledgerInfo,
Hash const& sorobanBasePrngSeed, uint32_t ledgerSeq,
Hash const& sorobanBasePrngSeed,
SorobanMetrics& sorobanMetrics, uint32_t ledgerSeq,
uint32_t ledgerVersion);

void applySorobanStage(Application& app, AbstractLedgerTxn& ltx,
Expand Down
12 changes: 7 additions & 5 deletions src/transactions/FeeBumpTransactionFrame.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,8 +106,9 @@ FeeBumpTransactionFrame::parallelApply(
ThreadEntryMap const& entryMap, // Must not be shared between threads!,
Config const& config, SorobanNetworkConfig const& sorobanConfig,
CxxLedgerInfo const& ledgerInfo, MutableTxResultPtr txResult,
Hash const& sorobanBasePrngSeed, TransactionMetaFrame& meta,
uint32_t ledgerSeq, uint32_t ledgerVersion) const
SorobanMetrics& sorobanMetrics, Hash const& sorobanBasePrngSeed,
TransactionMetaFrame& meta, uint32_t ledgerSeq,
uint32_t ledgerVersion) const
{
try
{
Expand All @@ -116,9 +117,10 @@ FeeBumpTransactionFrame::parallelApply(
// Note that even after updateResult is called here, feeCharged will not
// be accurate for Soroban transactions until
// FeeBumpTransactionFrame::processPostApply is called.
auto res = mInnerTx->parallelApply(
entryMap, config, sorobanConfig, ledgerInfo, txResult,
sorobanBasePrngSeed, meta, ledgerSeq, ledgerVersion);
auto res = mInnerTx->parallelApply(entryMap, config, sorobanConfig,
ledgerInfo, txResult, sorobanMetrics,
sorobanBasePrngSeed, meta, ledgerSeq,
ledgerVersion);
FeeBumpMutableTransactionResult::updateResult(mInnerTx, *txResult);
return res;
}
Expand Down
5 changes: 3 additions & 2 deletions src/transactions/FeeBumpTransactionFrame.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,9 @@ class FeeBumpTransactionFrame : public TransactionFrameBase
ThreadEntryMap const& entryMap, // Must not be shared between threads!,
Config const& config, SorobanNetworkConfig const& sorobanConfig,
CxxLedgerInfo const& ledgerInfo, MutableTxResultPtr resPayload,
Hash const& sorobanBasePrngSeed, TransactionMetaFrame& meta,
uint32_t ledgerSeq, uint32_t ledgerVersion) const override;
SorobanMetrics& sorobanMetrics, Hash const& sorobanBasePrngSeed,
TransactionMetaFrame& meta, uint32_t ledgerSeq,
uint32_t ledgerVersion) const override;

bool apply(Application& app, AbstractLedgerTxn& ltx,
TransactionMetaFrame& meta, MutableTxResultPtr txResult,
Expand Down
180 changes: 14 additions & 166 deletions src/transactions/InvokeHostFunctionOpFrame.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,154 +106,6 @@ metricsEvent(bool success, std::string&& topic, uint64_t value)

} // namespace

// TODO: Deduplicate. Maybe just include both for v23, and then remove the
// destructor after we're on v23.
// The ledger level metrics should not be observable.
class ParallelHostFunctionMetrics : public SorobanOpMetrics
{
public:
uint32_t mReadEntry{0};
uint32_t mWriteEntry{0};

uint32_t mLedgerReadByte{0};
uint32_t mLedgerWriteByte{0};

uint32_t mReadKeyByte{0};
uint32_t mWriteKeyByte{0};

uint32_t mReadDataByte{0};
uint32_t mWriteDataByte{0};

uint32_t mReadCodeByte{0};
uint32_t mWriteCodeByte{0};

uint32_t mEmitEvent{0};
uint32_t mEmitEventByte{0};

// host runtime metrics
uint64_t mCpuInsn{0};
uint64_t mMemByte{0};
uint64_t mInvokeTimeNsecs{0};
uint64_t mCpuInsnExclVm{0};
uint64_t mInvokeTimeNsecsExclVm{0};

// max single entity size metrics
uint32_t mMaxReadWriteKeyByte{0};
uint32_t mMaxReadWriteDataByte{0};
uint32_t mMaxReadWriteCodeByte{0};
uint32_t mMaxEmitEventByte{0};

std::chrono::nanoseconds mHostFuncOpExecNsecs{0};

bool mSuccess{false};

void
noteReadEntry(bool isCodeEntry, uint32_t keySize, uint32_t entrySize)
{
mReadEntry++;
mReadKeyByte += keySize;
mMaxReadWriteKeyByte = std::max(mMaxReadWriteKeyByte, keySize);
mLedgerReadByte += entrySize;
if (isCodeEntry)
{
mReadCodeByte += entrySize;
mMaxReadWriteCodeByte = std::max(mMaxReadWriteCodeByte, entrySize);
}
else
{
mReadDataByte += entrySize;
mMaxReadWriteDataByte = std::max(mMaxReadWriteDataByte, entrySize);
}
}

void
noteWriteEntry(bool isCodeEntry, uint32_t keySize, uint32_t entrySize)
{
mWriteEntry++;
mMaxReadWriteKeyByte = std::max(mMaxReadWriteKeyByte, keySize);
mLedgerWriteByte += entrySize;
if (isCodeEntry)
{
mWriteCodeByte += entrySize;
mMaxReadWriteCodeByte = std::max(mMaxReadWriteCodeByte, entrySize);
}
else
{
mWriteDataByte += entrySize;
mMaxReadWriteDataByte = std::max(mMaxReadWriteDataByte, entrySize);
}
}

void
updateSorobanMetrics(SorobanMetrics& metrics) override
{
metrics.mHostFnOpReadEntry.Mark(mReadEntry);
metrics.mHostFnOpWriteEntry.Mark(mWriteEntry);

metrics.mHostFnOpReadKeyByte.Mark(mReadKeyByte);
metrics.mHostFnOpWriteKeyByte.Mark(mWriteKeyByte);

metrics.mHostFnOpReadLedgerByte.Mark(mLedgerReadByte);
metrics.mHostFnOpReadDataByte.Mark(mReadDataByte);
metrics.mHostFnOpReadCodeByte.Mark(mReadCodeByte);

metrics.mHostFnOpWriteLedgerByte.Mark(mLedgerWriteByte);
metrics.mHostFnOpWriteDataByte.Mark(mWriteDataByte);
metrics.mHostFnOpWriteCodeByte.Mark(mWriteCodeByte);

metrics.mHostFnOpEmitEvent.Mark(mEmitEvent);
metrics.mHostFnOpEmitEventByte.Mark(mEmitEventByte);

metrics.mHostFnOpCpuInsn.Mark(mCpuInsn);
metrics.mHostFnOpMemByte.Mark(mMemByte);
metrics.mHostFnOpInvokeTimeNsecs.Update(
std::chrono::nanoseconds(mInvokeTimeNsecs));
metrics.mHostFnOpCpuInsnExclVm.Mark(mCpuInsnExclVm);
metrics.mHostFnOpInvokeTimeNsecsExclVm.Update(
std::chrono::nanoseconds(mInvokeTimeNsecsExclVm));
metrics.mHostFnOpInvokeTimeFsecsCpuInsnRatio.Update(
mInvokeTimeNsecs * 1000000 / std::max(mCpuInsn, uint64_t(1)));
metrics.mHostFnOpInvokeTimeFsecsCpuInsnRatioExclVm.Update(
mInvokeTimeNsecsExclVm * 1000000 /
std::max(mCpuInsnExclVm, uint64_t(1)));

metrics.mHostFnOpExec.Update(mHostFuncOpExecNsecs);

metrics.mHostFnOpMaxRwKeyByte.Mark(mMaxReadWriteKeyByte);
metrics.mHostFnOpMaxRwDataByte.Mark(mMaxReadWriteDataByte);
metrics.mHostFnOpMaxRwCodeByte.Mark(mMaxReadWriteCodeByte);
metrics.mHostFnOpMaxEmitEventByte.Mark(mMaxEmitEventByte);

if (mSuccess)
{
metrics.mHostFnOpSuccess.Mark();
}
else
{
metrics.mHostFnOpFailure.Mark();
}
}
};

struct CallTimer
{
private:
ParallelHostFunctionMetrics& mMetrics;
std::chrono::steady_clock::time_point mStartTime;

public:
CallTimer(ParallelHostFunctionMetrics& metrics)
: mMetrics(metrics), mStartTime(std::chrono::steady_clock::now())
{
}

~CallTimer()
{
mMetrics.mHostFuncOpExecNsecs =
std::chrono::steady_clock::now() - mStartTime;
}
};

struct HostFunctionMetrics
{
SorobanMetrics& mMetrics;
Expand Down Expand Up @@ -406,11 +258,10 @@ InvokeHostFunctionOpFrame::doApply(AbstractLedgerTxn& ltx,
"InvokeHostFunctionOpFrame::doApply needs Config and base PRNG seed");
}

template <typename MetricsT>
void
InvokeHostFunctionOpFrame::maybePopulateDiagnosticEvents(
Config const& cfg, InvokeHostFunctionOutput const& output,
MetricsT const& metrics, SorobanTxData& sorobanData) const
HostFunctionMetrics const& metrics, SorobanTxData& sorobanData) const
{
if (cfg.ENABLE_SOROBAN_DIAGNOSTIC_EVENTS)
{
Expand Down Expand Up @@ -479,19 +330,16 @@ InvokeHostFunctionOpFrame::doApplyParallel(
ThreadEntryMap const& entryMap, // Must not be shared between threads
Config const& appConfig, SorobanNetworkConfig const& sorobanConfig,
Hash const& sorobanBasePrngSeed, CxxLedgerInfo const& ledgerInfo,
OperationResult& res, SorobanTxData& sorobanData, uint32_t ledgerSeq,
SorobanMetrics& sorobanMetrics, OperationResult& res,
SorobanTxData& sorobanData, uint32_t ledgerSeq,
uint32_t ledgerVersion) const
{
ZoneNamedN(applyZone, "InvokeHostFunctionOpFrame apply", true);

std::vector<LedgerEntryChange> changes;

auto metricsPtr = std::shared_ptr<ParallelHostFunctionMetrics>(
new ParallelHostFunctionMetrics());
releaseAssert(metricsPtr);
auto& metrics = *metricsPtr;

CallTimer callTimer(metrics);
HostFunctionMetrics metrics(sorobanMetrics);
auto timeScope = metrics.getExecTimer();

// Get the entries for the footprint
rust::Vec<CxxBuf> ledgerEntryCxxBufs;
Expand Down Expand Up @@ -622,13 +470,13 @@ InvokeHostFunctionOpFrame::doApplyParallel(
if (!addReads(footprint.readOnly))
{
// Error code set in addReads
return {false, {}, metricsPtr};
return {false, {}};
}

if (!addReads(footprint.readWrite))
{
// Error code set in addReads
return {false, {}, metricsPtr};
return {false, {}};
}

rust::Vec<CxxBuf> authEntryCxxBufs;
Expand Down Expand Up @@ -703,7 +551,7 @@ InvokeHostFunctionOpFrame::doApplyParallel(
{
innerResult(res).code(INVOKE_HOST_FUNCTION_TRAPPED);
}
return {false, {}, metricsPtr};
return {false, {}};
}

// Keep track of updates we need to make
Expand All @@ -721,7 +569,7 @@ InvokeHostFunctionOpFrame::doApplyParallel(
sorobanData))
{
innerResult(res).code(INVOKE_HOST_FUNCTION_RESOURCE_LIMIT_EXCEEDED);
return {false, {}, metricsPtr};
return {false, {}};
}

auto lk = LedgerEntryKey(le);
Expand All @@ -744,7 +592,7 @@ InvokeHostFunctionOpFrame::doApplyParallel(
makeU64SCVal(resources.writeBytes)});
innerResult(res).code(
INVOKE_HOST_FUNCTION_RESOURCE_LIMIT_EXCEEDED);
return {false, {}, metricsPtr};
return {false, {}};
}
}

Expand Down Expand Up @@ -817,7 +665,7 @@ InvokeHostFunctionOpFrame::doApplyParallel(
{makeU64SCVal(metrics.mEmitEventByte),
makeU64SCVal(sorobanConfig.txMaxContractEventsSizeBytes())});
innerResult(res).code(INVOKE_HOST_FUNCTION_RESOURCE_LIMIT_EXCEEDED);
return {false, {}, metricsPtr};
return {false, {}};
}
ContractEvent evt;
xdr::xdr_from_opaque(buf.data, evt);
Expand All @@ -834,15 +682,15 @@ InvokeHostFunctionOpFrame::doApplyParallel(
{makeU64SCVal(metrics.mEmitEventByte),
makeU64SCVal(sorobanConfig.txMaxContractEventsSizeBytes())});
innerResult(res).code(INVOKE_HOST_FUNCTION_RESOURCE_LIMIT_EXCEEDED);
return {false, {}, metricsPtr};
return {false, {}};
}

if (!sorobanData.consumeRefundableSorobanResources(
metrics.mEmitEventByte, out.rent_fee, ledgerVersion, sorobanConfig,
appConfig, mParentTx))
{
innerResult(res).code(INVOKE_HOST_FUNCTION_INSUFFICIENT_REFUNDABLE_FEE);
return {false, {}, metricsPtr};
return {false, {}};
}

xdr::xdr_from_opaque(out.result_value.data, success.returnValue);
Expand All @@ -853,7 +701,7 @@ InvokeHostFunctionOpFrame::doApplyParallel(
sorobanData.setReturnValue(success.returnValue);
metrics.mSuccess = true;

return {true, opEntryMap, metricsPtr};
return {true, opEntryMap};
}

bool
Expand Down
8 changes: 3 additions & 5 deletions src/transactions/InvokeHostFunctionOpFrame.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,9 @@ class InvokeHostFunctionOpFrame : public OperationFrame
return res.tr().invokeHostFunctionResult();
}

// TODO: Temporarily templated while we have both HostFunctionMetrics and
// ParallelHostFunctionMetrics
template <typename MetricsT>
void maybePopulateDiagnosticEvents(Config const& cfg,
InvokeHostFunctionOutput const& output,
MetricsT const& metrics,
HostFunctionMetrics const& metrics,
SorobanTxData& sorobanData) const;

InvokeHostFunctionOp const& mInvokeHostFunction;
Expand Down Expand Up @@ -58,7 +55,8 @@ class InvokeHostFunctionOpFrame : public OperationFrame
ThreadEntryMap const& entryMap, // Must not be shared between threads!
Config const& appConfig, SorobanNetworkConfig const& sorobanConfig,
Hash const& sorobanBasePrngSeed, CxxLedgerInfo const& ledgerInfo,
OperationResult& res, SorobanTxData& sorobanData, uint32_t ledgerSeq,
SorobanMetrics& sorobanMetrics, OperationResult& res,
SorobanTxData& sorobanData, uint32_t ledgerSeq,
uint32_t ledgerVersion) const override;

void
Expand Down
Loading

0 comments on commit 8d4c7df

Please sign in to comment.