Elide ALL commands that we are not trying to trace.#1883
Elide ALL commands that we are not trying to trace.#1883AWoloszyn merged 1 commit intogoogle:masterfrom
Conversation
gapii/cc/call_observer.cpp
Outdated
| } | ||
|
|
||
| void CallObserver::exit() { | ||
| if (!mSpy->should_trace(mApi)) { |
There was a problem hiding this comment.
This scares me. If the should_trace return value changes between enter and exit then You're Going To Have A Bad Time.
Are there cases where should_trace should change for the lifetime of a CallObserver? If not, can we fetch it once and store it as a member?
There was a problem hiding this comment.
There are no cases (that I know) that would make this change during a call.
The only place we change ShouldTrace is during MEC start
There was a problem hiding this comment.
I can definitely store it off as a member, that will prevent this from accidentally happening.
gapii/cc/spy_base.h
Outdated
| return should_trace(api) ? mEncoder : mNullEncoder; | ||
| } | ||
|
|
||
| std::shared_ptr<gapii::PackEncoder> parentEncoder(uint8_t api, CallObserver* parent) { |
There was a problem hiding this comment.
This feels like a bit of a clunky method. There's nothing particularly parent-specific about the implementation, and the name has no real indication of what it does.
I wonder if it would be clearer to just expose mNullEncoder and let the caller do this logic. This would also deobfuscate the call to should_trace.
We were ignoring some of the data, but not all of it. We would end up adding the commands to the tree, but no observations, this caused havok with the new memory work, since replaying those commands would cause OOB memory accesses.
2 participants
We were ignoring some of the data, but not all of it. We would
end up adding the commands to the tree, but no observations,
this caused havok with the new memory work, since replaying
those commands would cause OOB memory accesses.