aboutsummaryrefslogtreecommitdiffhomepage
path: root/tensorflow
diff options
context:
space:
mode:
authorGravatar A. Unique TensorFlower <gardener@tensorflow.org>2018-03-14 13:47:22 -0700
committerGravatar TensorFlower Gardener <gardener@tensorflow.org>2018-03-14 13:55:47 -0700
commit16632df29d09b3bbe5cc75df39f5ec720225e6d1 (patch)
tree3dfe1ffb7d5c8a4fc0cb6f9f8f6ae1fc4145464a /tensorflow
parent4ff81a7d891c2affadb9c9068cf37fdb109c0e77 (diff)
Remove nsync header files from tensorflow/core/platform/default/mutex.h
The implementation of tensorflow/core/platform/default/mutex.h uses the nsync library, so mutex.h has included nsync header files. This has been awkward, because each TensorFlow build (bazel, cmake, make, plus the instructions for compiling individual custom ops, on all the various platforms) has needed to handle the include paths correctly, reaching into a package that is downloaded separately from TensorFlow itself. This change avoids that awkwardness, instead taking on two different irritations: - mutex.h now defines two structs that are large enough and aligned enough to contain an nsync_mu and an nsync_cv. This is an abstraction violation, because TensorFlow's source should not need to know how big these data structures are. However, this is unlikely to cause problems because: 1) this is checked by a static assertion in mutex.cc, so we will notice immediately should a change be needed, and 2) this will likely never fail because we have no intent of allowing nsync's data strcutures to get bigger. - The methods of mutex and condition_variable can no longer be inlined, because that too would require mutex.h to include the nsync header files. (Or we'd need to declare the nsync functions directly in mutex.h, which would be another abstraction violation.) However, this is a small imposition because the overhead of a procedure call is typically small. The assumption behind this CL is that these irritations are less important than the ongoing frustration of maintaining the complex include path in multiple build systems. PiperOrigin-RevId: 189079523
Diffstat (limited to 'tensorflow')
-rw-r--r--tensorflow/compiler/xla/service/interpreter/BUILD5
-rw-r--r--tensorflow/contrib/android/cmake/CMakeLists.txt1
-rw-r--r--tensorflow/contrib/cmake/tf_shared_lib.cmake7
-rw-r--r--tensorflow/contrib/makefile/proto_text_cc_files.txt1
-rw-r--r--tensorflow/core/BUILD1
-rw-r--r--tensorflow/core/platform/default/mutex.cc89
-rw-r--r--tensorflow/core/platform/default/mutex.h54
-rw-r--r--tensorflow/python/platform/sysconfig.py1
-rw-r--r--tensorflow/tensorflow.bzl17
-rw-r--r--tensorflow/tools/pip_package/setup.py3
10 files changed, 121 insertions, 58 deletions
diff --git a/tensorflow/compiler/xla/service/interpreter/BUILD b/tensorflow/compiler/xla/service/interpreter/BUILD
index 0819ab3b90..0db3863f24 100644
--- a/tensorflow/compiler/xla/service/interpreter/BUILD
+++ b/tensorflow/compiler/xla/service/interpreter/BUILD
@@ -63,10 +63,7 @@ cc_library(
name = "platform_id",
srcs = ["platform_id.cc"],
hdrs = ["platform_id.h"],
- deps = [
- "@nsync//:nsync_headers",
- "//tensorflow/core:stream_executor_headers_lib",
- ] + if_static(
+ deps = ["//tensorflow/core:stream_executor_headers_lib"] + if_static(
["@protobuf_archive//:protobuf"],
["@protobuf_archive//:protobuf_headers"],
),
diff --git a/tensorflow/contrib/android/cmake/CMakeLists.txt b/tensorflow/contrib/android/cmake/CMakeLists.txt
index a115d1610e..ecf1a103d2 100644
--- a/tensorflow/contrib/android/cmake/CMakeLists.txt
+++ b/tensorflow/contrib/android/cmake/CMakeLists.txt
@@ -75,7 +75,6 @@ target_link_libraries(tensorflow_inference
include_directories(
${PREBUILT_DIR}/proto
${PREBUILT_DIR}/protobuf/include
- ${PREBUILT_DIR}/nsync/public
${TENSORFLOW_ROOT_DIR}/tensorflow/contrib/makefile/downloads/eigen
${TENSORFLOW_ROOT_DIR}
${CMAKE_CURRENT_SOURCE_DIR}/..)
diff --git a/tensorflow/contrib/cmake/tf_shared_lib.cmake b/tensorflow/contrib/cmake/tf_shared_lib.cmake
index 6d36d5fc5c..9738bbeb9a 100644
--- a/tensorflow/contrib/cmake/tf_shared_lib.cmake
+++ b/tensorflow/contrib/cmake/tf_shared_lib.cmake
@@ -100,8 +100,7 @@ if(WIN32)
endif(WIN32)
target_include_directories(tensorflow PUBLIC
- $<INSTALL_INTERFACE:include/>
- $<INSTALL_INTERFACE:include/external/nsync/public>)
+ $<INSTALL_INTERFACE:include/>)
install(TARGETS tensorflow EXPORT tensorflow_export
RUNTIME DESTINATION bin
@@ -133,10 +132,6 @@ install(DIRECTORY ${tensorflow_source_dir}/tensorflow/stream_executor/
install(DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/protobuf/src/protobuf/src/google/
DESTINATION include/google
FILES_MATCHING PATTERN "*.h")
-# nsync headers
-install(DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/external/nsync/
- DESTINATION include/external/nsync
- FILES_MATCHING PATTERN "*.h")
# Eigen directory
install(DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/eigen/src/eigen/Eigen/
DESTINATION include/Eigen)
diff --git a/tensorflow/contrib/makefile/proto_text_cc_files.txt b/tensorflow/contrib/makefile/proto_text_cc_files.txt
index d56e388477..77c936d8c5 100644
--- a/tensorflow/contrib/makefile/proto_text_cc_files.txt
+++ b/tensorflow/contrib/makefile/proto_text_cc_files.txt
@@ -17,6 +17,7 @@ tensorflow/core/platform/env_time.cc
tensorflow/core/platform/setround.cc
tensorflow/core/platform/denormal.cc
tensorflow/core/platform/default/tracing.cc
+tensorflow/core/platform/default/mutex.cc
tensorflow/core/platform/default/logging.cc
tensorflow/core/platform/cpu_info.cc
tensorflow/core/lib/wav/wav_io.cc
diff --git a/tensorflow/core/BUILD b/tensorflow/core/BUILD
index 9f5b8027f3..213315f40e 100644
--- a/tensorflow/core/BUILD
+++ b/tensorflow/core/BUILD
@@ -1958,7 +1958,6 @@ cc_header_only_library(
deps = [
":framework",
":reader_base",
- "@nsync//:nsync_headers",
],
)
diff --git a/tensorflow/core/platform/default/mutex.cc b/tensorflow/core/platform/default/mutex.cc
new file mode 100644
index 0000000000..79830a4738
--- /dev/null
+++ b/tensorflow/core/platform/default/mutex.cc
@@ -0,0 +1,89 @@
+/* Copyright 2018 The TensorFlow Authors. All Rights Reserved.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+==============================================================================*/
+
+#include "tensorflow/core/platform/mutex.h"
+#include <chrono>
+#include <condition_variable>
+#include "nsync_cv.h"
+#include "nsync_mu.h"
+
+namespace tensorflow {
+
+// Check that the external_mu_space struct used to reserve space for the mutex
+// in tensorflow::mutex is big enough.
+static_assert(sizeof(nsync::nsync_mu) <= sizeof(mutex::external_mu_space),
+ "tensorflow::mutex::external_mu_space needs to be bigger");
+
+// Cast a pointer to mutex::external_mu_space to a pointer to the mutex mutex
+// representation. This is done so that the header files for nsync_mu do not
+// need to be included in every file that uses tensorflow's mutex.
+static inline nsync::nsync_mu *mu_cast(mutex::external_mu_space *mu) {
+ return reinterpret_cast<nsync::nsync_mu *>(mu);
+}
+
+mutex::mutex() { nsync::nsync_mu_init(mu_cast(&mu_)); }
+
+void mutex::lock() { nsync::nsync_mu_lock(mu_cast(&mu_)); }
+
+bool mutex::try_lock() { return nsync::nsync_mu_trylock(mu_cast(&mu_)) != 0; };
+
+void mutex::unlock() { nsync::nsync_mu_unlock(mu_cast(&mu_)); }
+
+void mutex::lock_shared() { nsync::nsync_mu_rlock(mu_cast(&mu_)); }
+
+bool mutex::try_lock_shared() {
+ return nsync::nsync_mu_rtrylock(mu_cast(&mu_)) != 0;
+};
+
+void mutex::unlock_shared() { nsync::nsync_mu_runlock(mu_cast(&mu_)); }
+
+// Check that the external_cv_space struct used to reserve space for the
+// condition variable in tensorflow::condition_variable is big enough.
+static_assert(
+ sizeof(nsync::nsync_cv) <= sizeof(condition_variable::external_cv_space),
+ "tensorflow::condition_variable::external_cv_space needs to be bigger");
+
+// Cast a pointer to mutex::external_cv_space to a pointer to the condition
+// variable representation. This is done so that the header files for nsync_mu
+// do not need to be included in every file that uses tensorflow's
+// condition_variable.
+static inline nsync::nsync_cv *cv_cast(
+ condition_variable::external_cv_space *cv) {
+ return reinterpret_cast<nsync::nsync_cv *>(cv);
+}
+
+condition_variable::condition_variable() {
+ nsync::nsync_cv_init(cv_cast(&cv_));
+}
+
+void condition_variable::wait(mutex_lock &lock) {
+ nsync::nsync_cv_wait(cv_cast(&cv_), mu_cast(&lock.mutex()->mu_));
+}
+
+std::cv_status condition_variable::wait_until_system_clock(
+ mutex_lock &lock,
+ const std::chrono::system_clock::time_point timeout_time) {
+ int r = nsync::nsync_cv_wait_with_deadline(
+ cv_cast(&cv_), mu_cast(&lock.mutex()->mu_), timeout_time, nullptr);
+ return r ? std::cv_status::timeout : std::cv_status::no_timeout;
+}
+
+void condition_variable::notify_one() { nsync::nsync_cv_signal(cv_cast(&cv_)); }
+
+void condition_variable::notify_all() {
+ nsync::nsync_cv_broadcast(cv_cast(&cv_));
+}
+
+} // namespace tensorflow
diff --git a/tensorflow/core/platform/default/mutex.h b/tensorflow/core/platform/default/mutex.h
index 044c754e80..a12d92795e 100644
--- a/tensorflow/core/platform/default/mutex.h
+++ b/tensorflow/core/platform/default/mutex.h
@@ -22,9 +22,8 @@ limitations under the License.
#include <chrono>
#include <condition_variable>
#include <mutex>
-#include "nsync_cv.h"
-#include "nsync_mu.h"
#include "tensorflow/core/platform/thread_annotations.h"
+
namespace tensorflow {
#undef mutex_lock
@@ -38,26 +37,26 @@ class condition_variable;
// lock.
class LOCKABLE mutex {
public:
- mutex() { nsync::nsync_mu_init(&mu_); }
- // The default implementation of nsync_mutex is safe to use after the linker
- // initializations
+ mutex();
+ // The default implementation of the underlying mutex is safe to use after
+ // the linker initialization to zero.
explicit mutex(LinkerInitialized x) {}
- void lock() EXCLUSIVE_LOCK_FUNCTION() { nsync::nsync_mu_lock(&mu_); }
- bool try_lock() EXCLUSIVE_TRYLOCK_FUNCTION(true) {
- return nsync::nsync_mu_trylock(&mu_) != 0;
- };
- void unlock() UNLOCK_FUNCTION() { nsync::nsync_mu_unlock(&mu_); }
+ void lock() EXCLUSIVE_LOCK_FUNCTION();
+ bool try_lock() EXCLUSIVE_TRYLOCK_FUNCTION(true);
+ void unlock() UNLOCK_FUNCTION();
+
+ void lock_shared() SHARED_LOCK_FUNCTION();
+ bool try_lock_shared() SHARED_TRYLOCK_FUNCTION(true);
+ void unlock_shared() UNLOCK_FUNCTION();
- void lock_shared() SHARED_LOCK_FUNCTION() { nsync::nsync_mu_rlock(&mu_); }
- bool try_lock_shared() SHARED_TRYLOCK_FUNCTION(true) {
- return nsync::nsync_mu_rtrylock(&mu_) != 0;
+ struct external_mu_space {
+ void* space[2];
};
- void unlock_shared() UNLOCK_FUNCTION() { nsync::nsync_mu_runlock(&mu_); }
private:
friend class condition_variable;
- nsync::nsync_mu mu_;
+ external_mu_space mu_;
};
// Mimic a subset of the std::unique_lock<tensorflow::mutex> functionality.
@@ -139,26 +138,29 @@ class SCOPED_LOCKABLE tf_shared_lock {
// Mimic std::condition_variable.
class condition_variable {
public:
- condition_variable() { nsync::nsync_cv_init(&cv_); }
+ condition_variable();
- void wait(mutex_lock& lock) {
- nsync::nsync_cv_wait(&cv_, &lock.mutex()->mu_);
- }
+ void wait(mutex_lock& lock);
template <class Rep, class Period>
std::cv_status wait_for(mutex_lock& lock,
std::chrono::duration<Rep, Period> dur) {
- int r = nsync::nsync_cv_wait_with_deadline(
- &cv_, &lock.mutex()->mu_, std::chrono::system_clock::now() + dur,
- nullptr);
- return r ? std::cv_status::timeout : std::cv_status::no_timeout;
+ return wait_until_system_clock(lock,
+ std::chrono::system_clock::now() + dur);
}
- void notify_one() { nsync::nsync_cv_signal(&cv_); }
- void notify_all() { nsync::nsync_cv_broadcast(&cv_); }
+ void notify_one();
+ void notify_all();
+
+ struct external_cv_space {
+ void* space[2];
+ };
private:
friend ConditionResult WaitForMilliseconds(mutex_lock* mu,
condition_variable* cv, int64 ms);
- nsync::nsync_cv cv_;
+ std::cv_status wait_until_system_clock(
+ mutex_lock& lock,
+ const std::chrono::system_clock::time_point timeout_time);
+ external_cv_space cv_;
};
inline ConditionResult WaitForMilliseconds(mutex_lock* mu,
diff --git a/tensorflow/python/platform/sysconfig.py b/tensorflow/python/platform/sysconfig.py
index 5c50fa023d..fdd2b903fc 100644
--- a/tensorflow/python/platform/sysconfig.py
+++ b/tensorflow/python/platform/sysconfig.py
@@ -68,7 +68,6 @@ def get_compile_flags():
"""
flags = []
flags.append('-I%s' % get_include())
- flags.append('-I%s/external/nsync/public' % get_include())
flags.append('-D_GLIBCXX_USE_CXX11_ABI=%d' % _CXX11_ABI_FLAG)
return flags
diff --git a/tensorflow/tensorflow.bzl b/tensorflow/tensorflow.bzl
index 818d67f7b5..ae305a28e0 100644
--- a/tensorflow/tensorflow.bzl
+++ b/tensorflow/tensorflow.bzl
@@ -1158,22 +1158,6 @@ def transitive_hdrs(name, deps=[], **kwargs):
# the libraries in deps.
def cc_header_only_library(name, deps=[], includes=[], **kwargs):
_transitive_hdrs(name=name + "_gather", deps=deps)
-
- # We could generalize the following, but rather than complicate things
- # here, we'll do the minimal use case for now, and hope bazel comes up
- # with a better solution before too long. We'd expect it to compute
- # the right include path by itself, but it doesn't, possibly because
- # _transitive_hdrs lost some information about the include path.
- if "@nsync//:nsync_headers" in deps:
- # Buiding tensorflow from @org_tensorflow finds this two up.
- nsynch = "../../external/nsync/public"
- # Building tensorflow from elsewhere finds it four up.
- # Note that native.repository_name() is not yet available in TF's Kokoro.
- if REPOSITORY_NAME != "@":
- nsynch = "../../" + nsynch
- includes = includes[:]
- includes.append(nsynch)
-
native.cc_library(name=name,
hdrs=[":" + name + "_gather"],
includes=includes,
@@ -1182,7 +1166,6 @@ def cc_header_only_library(name, deps=[], includes=[], **kwargs):
def tf_custom_op_library_additional_deps():
return [
"@protobuf_archive//:protobuf_headers",
- "@nsync//:nsync_headers",
clean_dep("//third_party/eigen3"),
clean_dep("//tensorflow/core:framework_headers_lib"),
]
diff --git a/tensorflow/tools/pip_package/setup.py b/tensorflow/tools/pip_package/setup.py
index 4b6f123daa..e1a5f091ba 100644
--- a/tensorflow/tools/pip_package/setup.py
+++ b/tensorflow/tools/pip_package/setup.py
@@ -200,8 +200,7 @@ headers = (list(find_files('*.h', 'tensorflow/core')) +
list(find_files('*.h', 'tensorflow/stream_executor')) +
list(find_files('*.h', 'google/protobuf_archive/src')) +
list(find_files('*', 'third_party/eigen3')) +
- list(find_files('*', 'external/eigen_archive')) +
- list(find_files('*.h', 'external/nsync/public')))
+ list(find_files('*', 'external/eigen_archive')))
setup(
name=project_name,