From fd4ece49c9ec143a9d689df6d8c1b26857936794 Mon Sep 17 00:00:00 2001 From: Sergei Lebedev <slebedev@google.com> Date: Wed, 28 Aug 2019 13:59:33 -0700 Subject: [PATCH] Removed StackFrame.func_start_lineno and Operation.traceback_with_start_lines They were only referenced inside tfprof and, as far as I can tell, never used in the tfprof UI. PiperOrigin-RevId: 265987217 --- tensorflow/python/debug/lib/debug_data.py | 2 +- .../framework/error_interpolation_test.py | 11 +++-- tensorflow/python/framework/ops.py | 12 +----- tensorflow/python/framework/ops_test.py | 22 ---------- tensorflow/python/profiler/pprof_profiler.py | 4 +- tensorflow/python/profiler/tfprof_logger.py | 5 ++- tensorflow/python/util/tf_stack.cc | 40 ++++++++----------- tensorflow/python/util/tf_stack.py | 36 +---------------- tensorflow/python/util/tf_stack_test.py | 5 +-- .../api/golden/v1/tensorflow.-operation.pbtxt | 4 -- .../api/golden/v2/tensorflow.-operation.pbtxt | 4 -- 11 files changed, 35 insertions(+), 110 deletions(-) diff --git a/tensorflow/python/debug/lib/debug_data.py b/tensorflow/python/debug/lib/debug_data.py index 15d91f4c1e8..ceabd2e86d4 100644 --- a/tensorflow/python/debug/lib/debug_data.py +++ b/tensorflow/python/debug/lib/debug_data.py @@ -670,7 +670,7 @@ class DebugDumpDir(object): self._node_traceback = {} if self._python_graph: for op in self._python_graph.get_operations(): - self._node_traceback[op.name] = op.traceback + self._node_traceback[op.name] = tuple(map(tuple, op.traceback)) @property def python_graph(self): diff --git a/tensorflow/python/framework/error_interpolation_test.py b/tensorflow/python/framework/error_interpolation_test.py index 2378d2d8264..0fc1a95955f 100644 --- a/tensorflow/python/framework/error_interpolation_test.py +++ b/tensorflow/python/framework/error_interpolation_test.py @@ -18,6 +18,7 @@ from __future__ import absolute_import from __future__ import division from __future__ import print_function +import collections import os import re @@ -28,18 +29,20 @@ from tensorflow.python.framework import test_util from tensorflow.python.framework import traceable_stack from tensorflow.python.ops import math_ops from tensorflow.python.platform import test -from tensorflow.python.util import tf_stack + +# A mock for ``tf_stack.StackFrame``. +StackFrame = collections.namedtuple( + "StackFrame", ["filename", "lineno", "name", "line"]) def _make_frame_with_filename(op, idx, filename): """Return a copy of an existing stack frame with a new filename.""" frame = op._traceback[idx] - return tf_stack.StackFrame( + return StackFrame( filename, frame.lineno, frame.name, - frame.globals, - frame.func_start_lineno) + frame.line) def _modify_op_stack_with_filenames(op, num_user_frames, user_filename, diff --git a/tensorflow/python/framework/ops.py b/tensorflow/python/framework/ops.py index cf25406932d..b0702ca0af3 100644 --- a/tensorflow/python/framework/ops.py +++ b/tensorflow/python/framework/ops.py @@ -2304,17 +2304,7 @@ class Operation(object): @property def traceback(self): """Returns the call stack from when this operation was constructed.""" - return tf_stack.convert_stack(self._traceback) - - @property - def traceback_with_start_lines(self): - """Same as traceback but includes start line of function definition. - - Returns: - A list of 5-tuples (filename, lineno, name, code, func_start_lineno). - """ - return tf_stack.convert_stack( - self._traceback, include_func_start_lineno=True) + return self._traceback def _set_attr(self, attr_name, attr_value): """Private method used to set an attribute in the node_def.""" diff --git a/tensorflow/python/framework/ops_test.py b/tensorflow/python/framework/ops_test.py index 8e97a783445..5399e23098b 100644 --- a/tensorflow/python/framework/ops_test.py +++ b/tensorflow/python/framework/ops_test.py @@ -3350,28 +3350,6 @@ class NameScopeTest(test_util.TensorFlowTestCase): self.assertRaisesRegexp(ValueError, "'_' is not a valid scope name", f) -class TracebackTest(test_util.TensorFlowTestCase): - - @test_util.run_deprecated_v1 - def testTracebackWithStartLines(self): - with self.cached_session() as sess: - a = constant_op.constant(2.0) - sess.run( - a, - options=config_pb2.RunOptions( - trace_level=config_pb2.RunOptions.FULL_TRACE)) - self.assertTrue(sess.graph.get_operations()) - - # Tests that traceback_with_start_lines is the same as traceback - # but includes one more element at the end. - for op in sess.graph.get_operations(): - self.assertEquals(len(op.traceback), len(op.traceback_with_start_lines)) - for frame, frame_with_start_line in zip( - op.traceback, op.traceback_with_start_lines): - self.assertEquals(5, len(frame_with_start_line)) - self.assertEquals(frame, frame_with_start_line[:-1]) - - class EnableEagerExecutionTest(test_util.TensorFlowTestCase): @test_util.run_v1_only("b/120545219") diff --git a/tensorflow/python/profiler/pprof_profiler.py b/tensorflow/python/profiler/pprof_profiler.py index 3892a0e724d..b8e38d53768 100644 --- a/tensorflow/python/profiler/pprof_profiler.py +++ b/tensorflow/python/profiler/pprof_profiler.py @@ -328,7 +328,7 @@ class PprofProfiler(object): # Call at current frame calls function at previous frame. prev_file_path = prev_stack_frame[0] prev_function = prev_stack_frame[2] - prev_function_start_line = prev_stack_frame[4] + prev_function_start_line = -1 curr_file_path = stack_frame[0] curr_line_number = stack_frame[1] @@ -371,7 +371,7 @@ class PprofProfiler(object): node_to_traceback = defaultdict(list) node_to_op_type = defaultdict(str) for op in self._graph.get_operations(): - node_to_traceback[op.name] = op.traceback_with_start_lines + node_to_traceback[op.name] = op.traceback node_to_op_type[op.name] = op.type def profile_data_generator(device_step_stats): diff --git a/tensorflow/python/profiler/tfprof_logger.py b/tensorflow/python/profiler/tfprof_logger.py index 75ee827adb5..8aff8cec085 100644 --- a/tensorflow/python/profiler/tfprof_logger.py +++ b/tensorflow/python/profiler/tfprof_logger.py @@ -113,13 +113,14 @@ def _get_logged_ops(graph, run_meta=None, add_trace=True, add_entry = True if add_trace: - for tb in op.traceback_with_start_lines: + for tb in op.traceback: trace = entry.code_def.traces.add() trace.file_id = _str_id(tb[0], string_to_id) if tb[0] else 0 trace.lineno = tb[1] if tb[1] else -1 trace.function_id = _str_id(tb[2], string_to_id) if tb[2] else 0 trace.line_id = _str_id(tb[3], string_to_id) if tb[3] else 0 - trace.func_start_line = tb[4] if tb[4] else -1 + # TODO(slebedev): remove this unused field from the proto. + trace.func_start_line = -1 add_entry = True if add_entry: diff --git a/tensorflow/python/util/tf_stack.cc b/tensorflow/python/util/tf_stack.cc index 2a4d2355e78..84c8ac08ee4 100644 --- a/tensorflow/python/util/tf_stack.cc +++ b/tensorflow/python/util/tf_stack.cc @@ -37,7 +37,6 @@ struct StackFrame { int lineno; py::str name; py::object globals; - int func_start_lineno; py::object line() const { static const auto* linecache = @@ -101,9 +100,7 @@ std::vector<StackFrame> ExtractStack(ssize_t limit, const py::list& mappers, } const auto& globals = py::reinterpret_borrow<py::object>(f->f_globals); - const int func_start_lineno = co->co_firstlineno; - ret.push_back({std::move(filename), lineno, std::move(name), globals, - func_start_lineno}); + ret.push_back({std::move(filename), lineno, std::move(name), globals}); } std::reverse(ret.begin(), ret.end()); @@ -115,14 +112,9 @@ std::vector<StackFrame> ExtractStack(ssize_t limit, const py::list& mappers, PYBIND11_MODULE(_tf_stack, m) { // TODO(slebedev): rename to FrameSummary to match Python 3.5+. py::class_<StackFrame>(m, "StackFrame") - .def(py::init<const py::str&, int, const py::str&, const py::object&, - int>()) .def_readonly("filename", &StackFrame::filename) .def_readonly("lineno", &StackFrame::lineno) .def_readonly("name", &StackFrame::name) - // TODO(slebedev): remove globals and make the constructor private. - .def_readonly("globals", &StackFrame::globals) - .def_readonly("func_start_lineno", &StackFrame::func_start_lineno) .def_property_readonly("line", &StackFrame::line) .def("__repr__", [](const StackFrame& self) { @@ -132,26 +124,28 @@ PYBIND11_MODULE(_tf_stack, m) { // For compatibility with the traceback module. .def("__getitem__", - [](const StackFrame& self, ssize_t index) -> py::object { - switch (index >= 0 ? index : 4 + index) { - case 0: - return self.filename; - case 1: - return py::cast(self.lineno); - case 2: - return self.name; - case 3: - return self.line(); - default: - throw py::index_error(); - } + [](const StackFrame& self, const py::object& index) -> py::object { + return py::make_tuple(self.filename, self.lineno, self.name, + self.line())[index]; }) .def("__len__", [](const StackFrame&) { return 4; // For compatibility with the traceback module. }); // TODO(slebedev): rename to StackSummary to match Python 3.5+. - py::bind_vector<std::vector<StackFrame>>(m, "Stack", py::module_local(true)); + py::bind_vector<std::vector<StackFrame>>(m, "Stack", py::module_local(true)) + // TODO(slebedev): upstream negative indexing support into pybind11. + .def( + "__getitem__", + [](const std::vector<StackFrame>& self, ssize_t index) { + const size_t eff_index = + index < 0 ? self.size() + index : static_cast<size_t>(index); + if (eff_index > self.size()) { + throw py::index_error(); + } + return self[eff_index]; + }, + py::return_value_policy::reference_internal); m.def("extract_stack", [](const py::object& limit, const py::list& mappers, const py::list& filters) { diff --git a/tensorflow/python/util/tf_stack.py b/tensorflow/python/util/tf_stack.py index bbc0170b13d..6ac8f3e1a2b 100644 --- a/tensorflow/python/util/tf_stack.py +++ b/tensorflow/python/util/tf_stack.py @@ -148,11 +148,8 @@ def extract_stack(limit=-1): limit: A limit on the number of frames to return. Returns: - A sequence of StackFrame objects - (filename, lineno, name, globals, func_start_lineno) - corresponding to the call stack of the current thread. The returned - tuples have the innermost stack frame at the end, unlike the Python - inspect module's stack() function. + A sequence of StackFrame objects (filename, lineno, name, line) + corresponding to the call stack of the current thread. """ # N.B ExtractStack in tf_stack.cc will drop this frame prior to # traversing the stack. @@ -163,32 +160,3 @@ def extract_stack(limit=-1): _source_filter_stacks[thread_key]) StackFrame = _tf_stack.StackFrame - - -def convert_stack(stack, include_func_start_lineno=False): - """Converts a stack extracted using extract_stack() to a traceback stack. - - Args: - stack: A sequence of StackFrame objects, - (filename, lineno, name, globals, func_start_lineno). - include_func_start_lineno: True if function start line number should be - included as the 5th entry in return tuples. - - Returns: - A tuple of n 4-tuples or 5-tuples - (filename, lineno, name, code, [optional: func_start_lineno]), where the - code tuple element is calculated from the corresponding elements of the - input tuple. - """ - def _tuple_generator(): # pylint: disable=missing-docstring - for frame in stack: - filename = frame.filename - lineno = frame.lineno - name = frame.name - line = frame.line - if include_func_start_lineno: - yield (filename, lineno, name, line, frame.func_start_lineno) - else: - yield (filename, lineno, name, line) - - return tuple(_tuple_generator()) diff --git a/tensorflow/python/util/tf_stack_test.py b/tensorflow/python/util/tf_stack_test.py index 4ab9af4d434..1395785df8c 100644 --- a/tensorflow/python/util/tf_stack_test.py +++ b/tensorflow/python/util/tf_stack_test.py @@ -36,7 +36,7 @@ class TFStackTest(test.TestCase): def testConsistencyWithTraceback(self): stack, expected_stack = extract_stack() for frame, expected in zip(stack, expected_stack): - self.assertEqual(frame, expected) + self.assertEqual(tuple(frame), expected) def testFormatStack(self): stack, expected_stack = extract_stack() @@ -46,9 +46,8 @@ class TFStackTest(test.TestCase): def extract_stack(limit=None): - convert = tf_stack.convert_stack # Both defined on the same line to produce identical stacks. - return convert(tf_stack.extract_stack(limit)), traceback.extract_stack(limit) + return tf_stack.extract_stack(limit), traceback.extract_stack(limit) if __name__ == "__main__": diff --git a/tensorflow/tools/api/golden/v1/tensorflow.-operation.pbtxt b/tensorflow/tools/api/golden/v1/tensorflow.-operation.pbtxt index 64240f70698..0f43a49ee96 100644 --- a/tensorflow/tools/api/golden/v1/tensorflow.-operation.pbtxt +++ b/tensorflow/tools/api/golden/v1/tensorflow.-operation.pbtxt @@ -38,10 +38,6 @@ tf_class { name: "traceback" mtype: "<type \'property\'>" } - member { - name: "traceback_with_start_lines" - mtype: "<type \'property\'>" - } member { name: "type" mtype: "<type \'property\'>" diff --git a/tensorflow/tools/api/golden/v2/tensorflow.-operation.pbtxt b/tensorflow/tools/api/golden/v2/tensorflow.-operation.pbtxt index 64240f70698..0f43a49ee96 100644 --- a/tensorflow/tools/api/golden/v2/tensorflow.-operation.pbtxt +++ b/tensorflow/tools/api/golden/v2/tensorflow.-operation.pbtxt @@ -38,10 +38,6 @@ tf_class { name: "traceback" mtype: "<type \'property\'>" } - member { - name: "traceback_with_start_lines" - mtype: "<type \'property\'>" - } member { name: "type" mtype: "<type \'property\'>"