From 3d3c4aaaedaf117b3b8cc9c30a594889094db6f1 Mon Sep 17 00:00:00 2001 From: George Karpenkov Date: Tue, 24 Nov 2020 13:11:27 -0800 Subject: [PATCH] [NFC] s/AbstractStackTrace/ManagedStackTrace There is nohing abstract about abstract stack trace: it assumes a particular storage, and it is passed by value everywhere, making extending this class virtually (pardon the pun) impossible. PiperOrigin-RevId: 344118317 Change-Id: Ic35f899d56f360eca45ac0b3f0752770926f6c22 --- tensorflow/c/eager/immediate_execution_operation.h | 6 +++--- tensorflow/core/common_runtime/eager/eager_operation.h | 8 ++++---- tensorflow/core/common_runtime/eager/execute_node.h | 4 ++-- tensorflow/core/util/BUILD | 6 +++--- .../{abstract_stack_trace.h => managed_stack_trace.h} | 4 ++-- tensorflow/python/eager/pywrap_tfe_src.cc | 2 +- tensorflow/python/util/stack_trace.h | 6 +++--- 7 files changed, 18 insertions(+), 18 deletions(-) rename tensorflow/core/util/{abstract_stack_trace.h => managed_stack_trace.h} (92%) diff --git a/tensorflow/c/eager/immediate_execution_operation.h b/tensorflow/c/eager/immediate_execution_operation.h index 7b68ec2c9f4..85af5a706e1 100644 --- a/tensorflow/c/eager/immediate_execution_operation.h +++ b/tensorflow/c/eager/immediate_execution_operation.h @@ -27,7 +27,7 @@ limitations under the License. #include "tensorflow/core/framework/types.pb.h" #include "tensorflow/core/platform/casts.h" #include "tensorflow/core/platform/status.h" -#include "tensorflow/core/util/abstract_stack_trace.h" +#include "tensorflow/core/util/managed_stack_trace.h" struct TFE_Op; @@ -48,10 +48,10 @@ class ImmediateExecutionOperation : public AbstractOperation { virtual Status OutputLength(const char* output_name, int* length) = 0; // Set stack trace to be used for potential async error reporting. - virtual void SetStackTrace(AbstractStackTrace stack_trace) = 0; + virtual void SetStackTrace(ManagedStackTrace stack_trace) = 0; // Returns the stack trace set by `SetStackTrace` if exists. - virtual absl::optional GetStackTrace() = 0; + virtual absl::optional GetStackTrace() = 0; // For LLVM style RTTI. static bool classof(const AbstractOperation* ptr) { diff --git a/tensorflow/core/common_runtime/eager/eager_operation.h b/tensorflow/core/common_runtime/eager/eager_operation.h index 2e35dd43582..5463158ae61 100644 --- a/tensorflow/core/common_runtime/eager/eager_operation.h +++ b/tensorflow/core/common_runtime/eager/eager_operation.h @@ -29,8 +29,8 @@ limitations under the License. #include "tensorflow/core/framework/cancellation.h" #include "tensorflow/core/framework/device_attributes.pb.h" #include "tensorflow/core/framework/op_def.pb.h" -#include "tensorflow/core/util/abstract_stack_trace.h" #include "tensorflow/core/util/device_name_utils.h" +#include "tensorflow/core/util/managed_stack_trace.h" namespace tensorflow { @@ -120,11 +120,11 @@ class EagerOperation : public ImmediateExecutionOperation { Status InputLength(const char* input_name, int* length) override; Status OutputLength(const char* output_name, int* length) override; - void SetStackTrace(AbstractStackTrace stack_trace) override { + void SetStackTrace(ManagedStackTrace stack_trace) override { stack_trace_ = stack_trace; } - absl::optional GetStackTrace() override { + absl::optional GetStackTrace() override { return stack_trace_; } @@ -225,7 +225,7 @@ class EagerOperation : public ImmediateExecutionOperation { // updated accordingly. VariantDevice device_; - absl::optional stack_trace_; + absl::optional stack_trace_; bool is_function_; // Conceptually const, but can't be because of Reset bool colocation_exempt_; CancellationManager* cancellation_manager_ = nullptr; // Not owned. diff --git a/tensorflow/core/common_runtime/eager/execute_node.h b/tensorflow/core/common_runtime/eager/execute_node.h index 6d11ecbf7a4..ed5f069f10a 100644 --- a/tensorflow/core/common_runtime/eager/execute_node.h +++ b/tensorflow/core/common_runtime/eager/execute_node.h @@ -151,7 +151,7 @@ class AsyncExecuteNode : public EagerNode { GraphCollector* graph_collector, CancellationManager* cancellation_manager, absl::Span retvals, - absl::optional stack_trace) + absl::optional stack_trace) : EagerNode(), ctx_(ctx), inputs_(inputs), @@ -233,7 +233,7 @@ class AsyncExecuteNode : public EagerNode { core::RefCountPtr kernel_; GraphCollector* graph_collector_; CancellationManager* const cancellation_manager_; - absl::optional stack_trace_; + absl::optional stack_trace_; absl::InlinedVector retvals_; }; diff --git a/tensorflow/core/util/BUILD b/tensorflow/core/util/BUILD index fefa99408b8..c85d6076205 100644 --- a/tensorflow/core/util/BUILD +++ b/tensorflow/core/util/BUILD @@ -74,7 +74,6 @@ filegroup( filegroup( name = "mobile_srcs_only_runtime", srcs = [ - "abstract_stack_trace.h", "batch_util.cc", "batch_util.h", "bcast.cc", @@ -97,6 +96,7 @@ filegroup( "example_proto_helper.h", "guarded_philox_random.cc", "guarded_philox_random.h", + "managed_stack_trace.h", "matmul_autotune.cc", "matmul_autotune.h", "matmul_bcast.h", @@ -327,7 +327,6 @@ filegroup( filegroup( name = "framework_srcs", srcs = [ - "abstract_stack_trace.h", "activation_mode.h", "batch_util.h", "bcast.h", @@ -340,6 +339,7 @@ filegroup( "example_proto_helper.h", "gpu_kernel_helper.h", "guarded_philox_random.h", + "managed_stack_trace.h", "matmul_autotune.h", "matmul_bcast.h", "mirror_pad_mode.h", @@ -470,7 +470,7 @@ cc_library( cc_library( name = "abstract_stack_trace", - hdrs = ["abstract_stack_trace.h"], + hdrs = ["managed_stack_trace.h"], visibility = [ "//tensorflow/c/eager:__pkg__", "//tensorflow/core:__pkg__", diff --git a/tensorflow/core/util/abstract_stack_trace.h b/tensorflow/core/util/managed_stack_trace.h similarity index 92% rename from tensorflow/core/util/abstract_stack_trace.h rename to tensorflow/core/util/managed_stack_trace.h index 442adc6f380..3cd02abbb22 100644 --- a/tensorflow/core/util/abstract_stack_trace.h +++ b/tensorflow/core/util/managed_stack_trace.h @@ -24,9 +24,9 @@ namespace tensorflow { // Language agnostic stack trace class. It only saves an id, and language // clients are responsible for managing the actual stack trace objects. -class AbstractStackTrace { +class ManagedStackTrace { public: - AbstractStackTrace(int id, std::vector (*to_stack_frames)(int)) + ManagedStackTrace(int id, std::vector (*to_stack_frames)(int)) : id_(id), to_stack_frames_(to_stack_frames) {} // Returns stack trace as a vector of `StackFrame`s. diff --git a/tensorflow/python/eager/pywrap_tfe_src.cc b/tensorflow/python/eager/pywrap_tfe_src.cc index 15d8ec9434b..74f28851445 100644 --- a/tensorflow/python/eager/pywrap_tfe_src.cc +++ b/tensorflow/python/eager/pywrap_tfe_src.cc @@ -42,7 +42,7 @@ limitations under the License. #include "tensorflow/core/platform/status.h" #include "tensorflow/core/platform/types.h" #include "tensorflow/core/profiler/lib/traceme.h" -#include "tensorflow/core/util/abstract_stack_trace.h" +#include "tensorflow/core/util/managed_stack_trace.h" #include "tensorflow/python/eager/pywrap_gradient_exclusions.h" #include "tensorflow/python/eager/pywrap_tensor.h" #include "tensorflow/python/eager/pywrap_tfe.h" diff --git a/tensorflow/python/util/stack_trace.h b/tensorflow/python/util/stack_trace.h index c12a841a1f3..0ee75af5a15 100644 --- a/tensorflow/python/util/stack_trace.h +++ b/tensorflow/python/util/stack_trace.h @@ -28,7 +28,7 @@ limitations under the License. #include "absl/base/optimization.h" #include "absl/container/inlined_vector.h" #include "absl/types/optional.h" -#include "tensorflow/core/util/abstract_stack_trace.h" +#include "tensorflow/core/util/managed_stack_trace.h" namespace tensorflow { @@ -158,9 +158,9 @@ extern StackTraceManager* const stack_trace_manager; // Note that the actual stack trace is kept in a circular buffer for string // conversion could fail if it's evicted before. // Python GIL must be acquired beforehand. -inline AbstractStackTrace GetStackTrace(int limit) { +inline ManagedStackTrace GetStackTrace(int limit) { DCheckPyGilStateForStackTrace(); - return AbstractStackTrace(stack_trace_manager->Capture(limit), [](int id) { + return ManagedStackTrace(stack_trace_manager->Capture(limit), [](int id) { PyGILState_STATE gstate = PyGILState_Ensure(); std::vector result = stack_trace_manager->Get(id)->ToStackFrames();