Fixes for innerloop trace management. #4913
Conversation
| { | ||
| NSLogEvents::TTDInnerLoopLogWriteEventLogEntry* evt = nullptr; | ||
| this->RecordGetInitializedEvent<NSLogEvents::TTDInnerLoopLogWriteEventLogEntry, NSLogEvents::EventKind::TTDInnerLoopLogWriteTag>(&evt); | ||
| byte buff[TTD_EVENT_PLUS_DATA_SIZE(NSLogEvents::TTDInnerLoopLogWriteEventLogEntry)]; |
There was a problem hiding this comment.
not clear why you're allocating a byte[] and then casting to EventLogEntry, instead of just allocating an EventLogEntry on stack. Latter would be more clear.
There was a problem hiding this comment.
The log layout is a packed setup where we expect the event payload data to immediately follow the event header. To ensure this layout is done correctly here we do the byte[] + manual casts instead of relying on the compiler to do contiguous layout on the stack. I'll add a comment here + ref to the other relevant code.
| return ContextAPIWrapper_NoRecord<true>([&](Js::ScriptContext * scriptContext) -> JsErrorCode { | ||
| if (!scriptContext->GetThreadContext()->IsRuntimeInTTDMode() || !scriptContext->GetThreadContext()->TTDLog->CanWriteInnerLoopTrace()) | ||
| { | ||
| return JsErrorDiagUnableToPerformAction; |
There was a problem hiding this comment.
Are callers of this method expecting such an error code & do they need to do anything special to account for it?
There was a problem hiding this comment.
Right now the caller just ignores the return code. However, the lack of an error code here is not inline with the form of the other API's so I wanted to get it added for cleanliness when I was adding the check code.
lib/Runtime/Debug/TTEventLog.h
Outdated
| bool CanWriteInnerLoopTrace() const; | ||
| bool SuppressDiagnosticTracesDuringInnerLoop() const; | ||
|
|
||
| void EmitLog(const char* emitUri, size_t emitUriLength, NSLogEvents::EventLogEntry* optInnerLoopEvent=nullptr); |
There was a problem hiding this comment.
nit: spacing around equals
| } | ||
| #endif | ||
| } | ||
| if (optInnerLoopEvent != nullptr) |
There was a problem hiding this comment.
nit: newline before if to make the separation more clear
| this->EmitLog(emitUri, emitUriLength, entry); | ||
| } | ||
|
|
||
| bool EventLog::CanWriteInnerLoopTrace() const |
There was a problem hiding this comment.
More pedantry than anything else but is the idea now that inner loop = record mode?
| } | ||
| if (optInnerLoopEvent != nullptr) | ||
| { | ||
| NSLogEvents::EventLogEntry_Emit(optInnerLoopEvent, this->m_eventListVTable, &writer, this->m_threadContext, NSTokens::Separator::BigSpaceSeparator); |
There was a problem hiding this comment.
What is this event supposed to signify?
mrkmarron
force-pushed
the
innerloopfixes
branch
from
e16f56d to
4b79e6e
Compare
|
@dotnet-bot test OSX static_osx_osx_debug please |
Merge pull request #4913 from mrkmarron:innerloopfixes Fixes around trace write for innerloop debugging scenarios.
…management. Merge pull request #4913 from mrkmarron:innerloopfixes Fixes around trace write for innerloop debugging scenarios.
5 participants
Fixes around trace write for innerloop debugging scenarios.