From 2a0c7ae7715fc7f05566614dcbc9a722f6e67f39 Mon Sep 17 00:00:00 2001 From: Derek Christ Date: Wed, 30 Jun 2021 16:25:38 +0200 Subject: [PATCH 1/4] Fix a memory leak in TraceAnalyzer A memory leak was fixed by replacing std::shared_ptr with std::weak_ptr in phase.h and phase.cpp for the pointer to its transaction. Not checking if the transaction is still valid is ok, because the transaction outlives the phase. --- .../traceAnalyzer/businessObjects/phases/phase.cpp | 12 ++++++------ DRAMSys/traceAnalyzer/businessObjects/phases/phase.h | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/DRAMSys/traceAnalyzer/businessObjects/phases/phase.cpp b/DRAMSys/traceAnalyzer/businessObjects/phases/phase.cpp index 1f0308f9..a5c6e5d3 100644 --- a/DRAMSys/traceAnalyzer/businessObjects/phases/phase.cpp +++ b/DRAMSys/traceAnalyzer/businessObjects/phases/phase.cpp @@ -129,22 +129,22 @@ QColor Phase::getColor(const TraceDrawingProperties &drawingProperties) const break; case ColorGrouping::Thread: return ColorGenerator::getColor(static_cast - (transaction->thread)); + (transaction.lock()->thread)); break; case ColorGrouping::Transaction: default: - return ColorGenerator::getColor(transaction->id); + return ColorGenerator::getColor(transaction.lock()->id); } } int Phase::getYVal(const TraceDrawingProperties &drawingProperties) const { if (getGranularity() == Granularity::Bankwise) - return drawingProperties.getYValOfBank(transaction->bank); + return drawingProperties.getYValOfBank(transaction.lock()->bank); else if (getGranularity() == Granularity::Groupwise) - return drawingProperties.getYValOfBankGroup(transaction->rank, transaction->bank); + return drawingProperties.getYValOfBankGroup(transaction.lock()->rank, transaction.lock()->bank); else // if (getGranularity() == Granularity::Rankwise) - return drawingProperties.getYValOfRank(transaction->rank); + return drawingProperties.getYValOfRank(transaction.lock()->rank); } Qt::BrushStyle Phase::getBrushStyle() const @@ -206,5 +206,5 @@ bool Phase::isCollapsed(const TraceDrawingProperties &drawingProperties) const if (dynamic_cast(this) != nullptr || dynamic_cast(this) != nullptr) return false; - return drawingProperties.getRankCollapsedState(transaction->rank); + return drawingProperties.getRankCollapsedState(transaction.lock()->rank); } diff --git a/DRAMSys/traceAnalyzer/businessObjects/phases/phase.h b/DRAMSys/traceAnalyzer/businessObjects/phases/phase.h index 986c0654..8e1bb888 100644 --- a/DRAMSys/traceAnalyzer/businessObjects/phases/phase.h +++ b/DRAMSys/traceAnalyzer/businessObjects/phases/phase.h @@ -80,7 +80,7 @@ protected: ID id; Timespan span; traceTime clk; - std::shared_ptr transaction; + std::weak_ptr transaction; std::vector spansOnCommandBus; std::shared_ptr spanOnDataBus; double hexagonHeight; From 7253b348008d4a70a062f7c244a0a2471db00913 Mon Sep 17 00:00:00 2001 From: Lukas Steiner Date: Fri, 2 Jul 2021 14:37:47 +0200 Subject: [PATCH 2/4] Fix arbitration delays in FIFO arbiter. --- DRAMSys/library/src/simulation/Arbiter.cpp | 115 ++++++++++++++------- DRAMSys/library/src/simulation/Arbiter.h | 3 + 2 files changed, 82 insertions(+), 36 deletions(-) diff --git a/DRAMSys/library/src/simulation/Arbiter.cpp b/DRAMSys/library/src/simulation/Arbiter.cpp index e4f0c515..703840f4 100644 --- a/DRAMSys/library/src/simulation/Arbiter.cpp +++ b/DRAMSys/library/src/simulation/Arbiter.cpp @@ -280,22 +280,25 @@ void ArbiterFifo::peqCallback(tlm_generic_payload &cbPayload, const tlm_phase &c tSocket[static_cast(threadId)]->nb_transport_bw(cbPayload, tPhase, tDelay); - pendingRequests[channelId].push(&cbPayload); + // TODO: move to REQ_ARBITRATION PEQ + payloadEventQueue.notify(cbPayload, REQ_ARBITRATION, arbitrationDelayFw); + //pendingRequests[channelId].push(&cbPayload); } else outstandingEndReq[threadId] = &cbPayload; - if (!channelIsBusy[channelId] && !pendingRequests[channelId].empty()) - { - channelIsBusy[channelId] = true; - - tlm_generic_payload &tPayload = *pendingRequests[channelId].front(); - pendingRequests[channelId].pop(); - tlm_phase tPhase = BEGIN_REQ; - sc_time tDelay = lastEndReq[channelId] == sc_time_stamp() ? tCK : SC_ZERO_TIME; - - iSocket[static_cast(channelId)]->nb_transport_fw(tPayload, tPhase, tDelay); - } + // TODO: move to REQ_ARBITRATION +// if (!channelIsBusy[channelId] && !pendingRequests[channelId].empty()) +// { +// channelIsBusy[channelId] = true; +// +// tlm_generic_payload &tPayload = *pendingRequests[channelId].front(); +// pendingRequests[channelId].pop(); +// tlm_phase tPhase = BEGIN_REQ; +// sc_time tDelay = lastEndReq[channelId] == sc_time_stamp() ? tCK : SC_ZERO_TIME; +// +// iSocket[static_cast(channelId)]->nb_transport_fw(tPayload, tPhase, tDelay); +// } } else if (cbPhase == END_REQ) // from memory controller { @@ -323,21 +326,23 @@ void ArbiterFifo::peqCallback(tlm_generic_payload &cbPayload, const tlm_phase &c iSocket[static_cast(channelId)]->nb_transport_fw(cbPayload, tPhase, tDelay); } - pendingResponses[threadId].push(&cbPayload); + // TODO: move to PEQ with RESP_ARBITRATION + payloadEventQueue.notify(cbPayload, RESP_ARBITRATION, arbitrationDelayBw); + //pendingResponses[threadId].push(&cbPayload); - if (!threadIsBusy[threadId]) - { - threadIsBusy[threadId] = true; - - tlm_generic_payload &tPayload = *pendingResponses[threadId].front(); - pendingResponses[threadId].pop(); - tlm_phase tPhase = BEGIN_RESP; - sc_time tDelay = lastEndResp[threadId] == sc_time_stamp() ? tCK : SC_ZERO_TIME; - - tlm_sync_enum returnValue = tSocket[static_cast(threadId)]->nb_transport_bw(tPayload, tPhase, tDelay); - if (returnValue == TLM_UPDATED) - payloadEventQueue.notify(tPayload, tPhase, tDelay); - } +// if (!threadIsBusy[threadId]) +// { +// threadIsBusy[threadId] = true; +// +// tlm_generic_payload &tPayload = *pendingResponses[threadId].front(); +// pendingResponses[threadId].pop(); +// tlm_phase tPhase = BEGIN_RESP; +// sc_time tDelay = lastEndResp[threadId] == sc_time_stamp() ? tCK : SC_ZERO_TIME; +// +// tlm_sync_enum returnValue = tSocket[static_cast(threadId)]->nb_transport_bw(tPayload, tPhase, tDelay); +// if (returnValue == TLM_UPDATED) +// payloadEventQueue.notify(tPayload, tPhase, tDelay); +// } } else if (cbPhase == END_RESP) // from initiator { @@ -358,17 +363,20 @@ void ArbiterFifo::peqCallback(tlm_generic_payload &cbPayload, const tlm_phase &c tSocket[static_cast(threadId)]->nb_transport_bw(tPayload, tPhase, tDelay); - if (!channelIsBusy[tChannelId]) - { - channelIsBusy[tChannelId] = true; + // TODO: move into REQ_ARBITRATION PEQ + payloadEventQueue.notify(tPayload, REQ_ARBITRATION, arbitrationDelayFw); - tPhase = BEGIN_REQ; - tDelay = lastEndReq[tChannelId] == sc_time_stamp() ? tCK : SC_ZERO_TIME; - - iSocket[static_cast(tChannelId)]->nb_transport_fw(tPayload, tPhase, tDelay); - } - else - pendingRequests[tChannelId].push(&tPayload); +// if (!channelIsBusy[tChannelId]) +// { +// channelIsBusy[tChannelId] = true; +// +// tPhase = BEGIN_REQ; +// tDelay = lastEndReq[tChannelId] == sc_time_stamp() ? tCK : SC_ZERO_TIME; +// +// iSocket[static_cast(tChannelId)]->nb_transport_fw(tPayload, tPhase, tDelay); +// } +// else +// pendingRequests[tChannelId].push(&tPayload); } else activeTransactions[threadId]--; @@ -387,6 +395,41 @@ void ArbiterFifo::peqCallback(tlm_generic_payload &cbPayload, const tlm_phase &c else threadIsBusy[threadId] = false; } + else if (cbPhase == REQ_ARBITRATION) + { + pendingRequests[channelId].push(&cbPayload); + + if (!channelIsBusy[channelId]) + { + channelIsBusy[channelId] = true; + + tlm_generic_payload &tPayload = *pendingRequests[channelId].front(); + pendingRequests[channelId].pop(); + tlm_phase tPhase = BEGIN_REQ; + sc_time tDelay = lastEndReq[channelId] == sc_time_stamp() ? tCK : SC_ZERO_TIME; + + iSocket[static_cast(channelId)]->nb_transport_fw(tPayload, tPhase, tDelay); + } + } + else if (cbPhase == RESP_ARBITRATION) + { + pendingResponses[threadId].push(&cbPayload); + + if (!threadIsBusy[threadId]) + { + threadIsBusy[threadId] = true; + + tlm_generic_payload &tPayload = *pendingResponses[threadId].front(); + pendingResponses[threadId].pop(); + tlm_phase tPhase = BEGIN_RESP; + sc_time tDelay = lastEndResp[threadId] == sc_time_stamp() ? tCK : SC_ZERO_TIME; + + tlm_sync_enum returnValue = tSocket[static_cast(threadId)]->nb_transport_bw(tPayload, tPhase, tDelay); + // Early completion from initiator + if (returnValue == TLM_UPDATED) + payloadEventQueue.notify(tPayload, tPhase, tDelay); + } + } else SC_REPORT_FATAL(0, "Payload event queue in arbiter was triggered with unknown phase"); } diff --git a/DRAMSys/library/src/simulation/Arbiter.h b/DRAMSys/library/src/simulation/Arbiter.h index 3746e784..3ff5370d 100644 --- a/DRAMSys/library/src/simulation/Arbiter.h +++ b/DRAMSys/library/src/simulation/Arbiter.h @@ -51,6 +51,9 @@ #include "AddressDecoder.h" #include "../common/dramExtensions.h" +DECLARE_EXTENDED_PHASE(REQ_ARBITRATION); +DECLARE_EXTENDED_PHASE(RESP_ARBITRATION); + class Arbiter : public sc_module { public: From 42939c057dfb5c4c922593e4a0fb45c6b0e23751 Mon Sep 17 00:00:00 2001 From: Lukas Steiner Date: Fri, 2 Jul 2021 15:11:50 +0200 Subject: [PATCH 3/4] Fix arbitration delay in reorder arbiter. --- DRAMSys/library/src/simulation/Arbiter.cpp | 135 +++++++-------------- 1 file changed, 46 insertions(+), 89 deletions(-) diff --git a/DRAMSys/library/src/simulation/Arbiter.cpp b/DRAMSys/library/src/simulation/Arbiter.cpp index 703840f4..f8851bbf 100644 --- a/DRAMSys/library/src/simulation/Arbiter.cpp +++ b/DRAMSys/library/src/simulation/Arbiter.cpp @@ -224,6 +224,7 @@ void ArbiterSimple::peqCallback(tlm_generic_payload &cbPayload, const tlm_phase sc_time tDelay = arbitrationDelayBw; tlm_sync_enum returnValue = tSocket[static_cast(threadId)]->nb_transport_bw(cbPayload, tPhase, tDelay); + // Early completion from initiator if (returnValue == TLM_UPDATED) payloadEventQueue.notify(cbPayload, tPhase, tDelay); threadIsBusy[threadId] = true; @@ -250,6 +251,7 @@ void ArbiterSimple::peqCallback(tlm_generic_payload &cbPayload, const tlm_phase sc_time tDelay = tCK + arbitrationDelayBw; tlm_sync_enum returnValue = tSocket[static_cast(threadId)]->nb_transport_bw(tPayload, tPhase, tDelay); + // Early completion from initiator if (returnValue == TLM_UPDATED) payloadEventQueue.notify(tPayload, tPhase, tDelay); } @@ -280,25 +282,10 @@ void ArbiterFifo::peqCallback(tlm_generic_payload &cbPayload, const tlm_phase &c tSocket[static_cast(threadId)]->nb_transport_bw(cbPayload, tPhase, tDelay); - // TODO: move to REQ_ARBITRATION PEQ payloadEventQueue.notify(cbPayload, REQ_ARBITRATION, arbitrationDelayFw); - //pendingRequests[channelId].push(&cbPayload); } else outstandingEndReq[threadId] = &cbPayload; - - // TODO: move to REQ_ARBITRATION -// if (!channelIsBusy[channelId] && !pendingRequests[channelId].empty()) -// { -// channelIsBusy[channelId] = true; -// -// tlm_generic_payload &tPayload = *pendingRequests[channelId].front(); -// pendingRequests[channelId].pop(); -// tlm_phase tPhase = BEGIN_REQ; -// sc_time tDelay = lastEndReq[channelId] == sc_time_stamp() ? tCK : SC_ZERO_TIME; -// -// iSocket[static_cast(channelId)]->nb_transport_fw(tPayload, tPhase, tDelay); -// } } else if (cbPhase == END_REQ) // from memory controller { @@ -326,23 +313,7 @@ void ArbiterFifo::peqCallback(tlm_generic_payload &cbPayload, const tlm_phase &c iSocket[static_cast(channelId)]->nb_transport_fw(cbPayload, tPhase, tDelay); } - // TODO: move to PEQ with RESP_ARBITRATION payloadEventQueue.notify(cbPayload, RESP_ARBITRATION, arbitrationDelayBw); - //pendingResponses[threadId].push(&cbPayload); - -// if (!threadIsBusy[threadId]) -// { -// threadIsBusy[threadId] = true; -// -// tlm_generic_payload &tPayload = *pendingResponses[threadId].front(); -// pendingResponses[threadId].pop(); -// tlm_phase tPhase = BEGIN_RESP; -// sc_time tDelay = lastEndResp[threadId] == sc_time_stamp() ? tCK : SC_ZERO_TIME; -// -// tlm_sync_enum returnValue = tSocket[static_cast(threadId)]->nb_transport_bw(tPayload, tPhase, tDelay); -// if (returnValue == TLM_UPDATED) -// payloadEventQueue.notify(tPayload, tPhase, tDelay); -// } } else if (cbPhase == END_RESP) // from initiator { @@ -363,20 +334,7 @@ void ArbiterFifo::peqCallback(tlm_generic_payload &cbPayload, const tlm_phase &c tSocket[static_cast(threadId)]->nb_transport_bw(tPayload, tPhase, tDelay); - // TODO: move into REQ_ARBITRATION PEQ payloadEventQueue.notify(tPayload, REQ_ARBITRATION, arbitrationDelayFw); - -// if (!channelIsBusy[tChannelId]) -// { -// channelIsBusy[tChannelId] = true; -// -// tPhase = BEGIN_REQ; -// tDelay = lastEndReq[tChannelId] == sc_time_stamp() ? tCK : SC_ZERO_TIME; -// -// iSocket[static_cast(tChannelId)]->nb_transport_fw(tPayload, tPhase, tDelay); -// } -// else -// pendingRequests[tChannelId].push(&tPayload); } else activeTransactions[threadId]--; @@ -389,6 +347,7 @@ void ArbiterFifo::peqCallback(tlm_generic_payload &cbPayload, const tlm_phase &c sc_time tDelay = tCK; tlm_sync_enum returnValue = tSocket[static_cast(threadId)]->nb_transport_bw(tPayload, tPhase, tDelay); + // Early completion from initiator if (returnValue == TLM_UPDATED) payloadEventQueue.notify(tPayload, tPhase, tDelay); } @@ -454,22 +413,10 @@ void ArbiterReorder::peqCallback(tlm_generic_payload &cbPayload, const tlm_phase tSocket[static_cast(threadId)]->nb_transport_bw(cbPayload, tPhase, tDelay); - pendingRequests[channelId].push(&cbPayload); + payloadEventQueue.notify(cbPayload, REQ_ARBITRATION, arbitrationDelayFw); } else outstandingEndReq[threadId] = &cbPayload; - - if (!channelIsBusy[channelId] && !pendingRequests[channelId].empty()) - { - channelIsBusy[channelId] = true; - - tlm_generic_payload &tPayload = *pendingRequests[channelId].front(); - pendingRequests[channelId].pop(); - tlm_phase tPhase = BEGIN_REQ; - sc_time tDelay = lastEndReq[channelId] == sc_time_stamp() ? tCK : SC_ZERO_TIME; - - iSocket[static_cast(channelId)]->nb_transport_fw(tPayload, tPhase, tDelay); - } } else if (cbPhase == END_REQ) // from memory controller { @@ -496,26 +443,7 @@ void ArbiterReorder::peqCallback(tlm_generic_payload &cbPayload, const tlm_phase iSocket[static_cast(channelId)]->nb_transport_fw(cbPayload, tPhase, tDelay); } - pendingResponses[threadId].insert(&cbPayload); - - if (!threadIsBusy[threadId]) - { - tlm_generic_payload &tPayload = **pendingResponses[threadId].begin(); - - if (DramExtension::getThreadPayloadID(tPayload) == nextThreadPayloadIDToReturn[threadId]) - { - nextThreadPayloadIDToReturn[threadId]++; - pendingResponses[threadId].erase(pendingResponses[threadId].begin()); - threadIsBusy[threadId] = true; - - tlm_phase tPhase = BEGIN_RESP; - sc_time tDelay = lastEndResp[threadId] == sc_time_stamp() ? tCK : SC_ZERO_TIME; - - tlm_sync_enum returnValue = tSocket[static_cast(threadId)]->nb_transport_bw(tPayload, tPhase, tDelay); - if (returnValue == TLM_UPDATED) - payloadEventQueue.notify(tPayload, tPhase, tDelay); - } - } + payloadEventQueue.notify(cbPayload, RESP_ARBITRATION, arbitrationDelayBw); } else if (cbPhase == END_RESP) // from initiator { @@ -536,17 +464,7 @@ void ArbiterReorder::peqCallback(tlm_generic_payload &cbPayload, const tlm_phase tSocket[static_cast(threadId)]->nb_transport_bw(tPayload, tPhase, tDelay); - if (!channelIsBusy[tChannelId]) - { - channelIsBusy[tChannelId] = true; - - tPhase = BEGIN_REQ; - tDelay = lastEndReq[tChannelId] == sc_time_stamp() ? tCK : SC_ZERO_TIME; - - iSocket[static_cast(tChannelId)]->nb_transport_fw(tPayload, tPhase, tDelay); - } - else - pendingRequests[tChannelId].push(&tPayload); + payloadEventQueue.notify(tPayload, REQ_ARBITRATION, arbitrationDelayFw); } else activeTransactions[threadId]--; @@ -563,12 +481,51 @@ void ArbiterReorder::peqCallback(tlm_generic_payload &cbPayload, const tlm_phase sc_time tDelay = tCK; tlm_sync_enum returnValue = tSocket[static_cast(threadId)]->nb_transport_bw(tPayload, tPhase, tDelay); + // Early completion from initiator if (returnValue == TLM_UPDATED) payloadEventQueue.notify(tPayload, tPhase, tDelay); } else - { threadIsBusy[threadId] = false; + } + else if (cbPhase == REQ_ARBITRATION) + { + pendingRequests[channelId].push(&cbPayload); + + if (!channelIsBusy[channelId]) + { + channelIsBusy[channelId] = true; + + tlm_generic_payload &tPayload = *pendingRequests[channelId].front(); + pendingRequests[channelId].pop(); + tlm_phase tPhase = BEGIN_REQ; + sc_time tDelay = lastEndReq[channelId] == sc_time_stamp() ? tCK : SC_ZERO_TIME; + + iSocket[static_cast(channelId)]->nb_transport_fw(tPayload, tPhase, tDelay); + } + } + else if (cbPhase == RESP_ARBITRATION) + { + pendingResponses[threadId].insert(&cbPayload); + + if (!threadIsBusy[threadId]) + { + tlm_generic_payload &tPayload = **pendingResponses[threadId].begin(); + + if (DramExtension::getThreadPayloadID(tPayload) == nextThreadPayloadIDToReturn[threadId]) + { + threadIsBusy[threadId] = true; + + nextThreadPayloadIDToReturn[threadId]++; + pendingResponses[threadId].erase(pendingResponses[threadId].begin()); + tlm_phase tPhase = BEGIN_RESP; + sc_time tDelay = lastEndResp[threadId] == sc_time_stamp() ? tCK : SC_ZERO_TIME; + + tlm_sync_enum returnValue = tSocket[static_cast(threadId)]->nb_transport_bw(tPayload, tPhase, tDelay); + // Early completion from initiator + if (returnValue == TLM_UPDATED) + payloadEventQueue.notify(tPayload, tPhase, tDelay); + } } } else From 51c30fe80470aa179893804e3623c5f0818265a3 Mon Sep 17 00:00:00 2001 From: Iron Prando Date: Wed, 14 Jul 2021 16:04:16 +0200 Subject: [PATCH 4/4] Adding argument parsing for metrics.py script - metric selection by name. Backward compatible. --- DRAMSys/traceAnalyzer/scripts/metrics.py | 69 +++++++++++++++++++++++- 1 file changed, 67 insertions(+), 2 deletions(-) diff --git a/DRAMSys/traceAnalyzer/scripts/metrics.py b/DRAMSys/traceAnalyzer/scripts/metrics.py index 92f1ad3d..cda9484c 100644 --- a/DRAMSys/traceAnalyzer/scripts/metrics.py +++ b/DRAMSys/traceAnalyzer/scripts/metrics.py @@ -909,6 +909,71 @@ def calculateMetrics(pathToTrace, selectedMetrics=[]): connection.close() return calculatedMetrics +def calculateMetricsFromFuncs(pathToTrace, selectedMetrics): + + calculatedMetrics = [] + connection = sqlite3.connect(pathToTrace) + + mcconfig = MCConfig(connection) + + print("================================") + print("Calculating metrics for {0}".format(pathToTrace)) + + print("Number of threads is {0}".format(len(getThreads(connection)))) + + if not selectedMetrics: + selectedMetrics = [0] * (len(metrics) + len(getThreads(connection))*len(threadMetrics) + 1) + for i in range(len(selectedMetrics)): + selectedMetrics[i] = True + + for metric in selectedMetrics: + mres = metric(connection) + mname = metric.__name__.replace("_", " ") + res = (mname, mres) + + if (metric.__name__ == "bank_overlap_ratio"): + values = mres.split(",") + nbanks = 0 + for v in values: + name = mname + " (" + str(nbanks) + " banks active)" + nbanks = nbanks + 1 + r = (name, float(v)) + calculatedMetrics.append(r) + else: + calculatedMetrics.append(res) + + print("{0}: {1}".format(res[0], res[1])) + + # refreshMissDecision(connection, calculatedMetrics) + connection.close() + return calculatedMetrics + +import argparse + if __name__ == "__main__": - path = sys.argv[1] - calculateMetrics(path) + """ + Only non-threaded metrics are implemented for selection through command line + """ + parser = argparse.ArgumentParser(description="Calculates metrics of a given .tdb file") + + parser.add_argument('path', type=str, help="The path to the .tdb file to be used") + + dic_metric_functions = {} + for m in metrics: + parser.add_argument("--"+m.__name__, action='store_true') + dic_metric_functions[m.__name__] = m + + arg_namespace = parser.parse_args(sys.argv[1:]) + + selected_metrics = [] + for k, v in arg_namespace.__dict__.items(): + if k == 'path': + continue + if v: + selected_metrics.append(dic_metric_functions[k]) + + + if selected_metrics == []: + calculateMetrics(arg_namespace.path) + else: + calculateMetricsFromFuncs(arg_namespace.path, selected_metrics)