Skip to content

Commit

Permalink
Do not create an extra default instance of T when constructing a Thre…
Browse files Browse the repository at this point in the history 
…adLocal<T>.

git-svn-id: http://googletest.googlecode.com/svn/trunk@732 861a406c-534a-0410-8894-cb66d6ee9925
  • Loading branch information
kosak@google.com committed Jul 19, 2015
1 parent 57bbc39 commit 0afa989
Show file tree
Hide file tree
Showing 2 changed files with 87 additions and 42 deletions.
88 changes: 78 additions & 10 deletions include/gtest/internal/gtest-port.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1838,8 +1838,9 @@ class ThreadWithParam : public ThreadWithParamBase {
template <typename T>
class ThreadLocal : public ThreadLocalBase {
public:
ThreadLocal() : default_() {}
explicit ThreadLocal(const T& value) : default_(value) {}
ThreadLocal() : default_factory_(new DefaultValueHolderFactory()) {}
explicit ThreadLocal(const T& value)
: default_factory_(new InstanceValueHolderFactory(value)) {}

~ThreadLocal() { ThreadLocalRegistry::OnThreadLocalDestroyed(this); }

Expand All @@ -1853,6 +1854,7 @@ class ThreadLocal : public ThreadLocalBase {
// knowing the type of T.
class ValueHolder : public ThreadLocalValueHolderBase {
public:
ValueHolder() : value_() {}
explicit ValueHolder(const T& value) : value_(value) {}

T* pointer() { return &value_; }
Expand All @@ -1869,10 +1871,42 @@ class ThreadLocal : public ThreadLocalBase {
}

virtual ThreadLocalValueHolderBase* NewValueForCurrentThread() const {
return new ValueHolder(default_);
return default_factory_->MakeNewHolder();
}

const T default_; // The default value for each thread.
class ValueHolderFactory {
public:
ValueHolderFactory() {}
virtual ~ValueHolderFactory() {}
virtual ValueHolder* MakeNewHolder() const = 0;

private:
GTEST_DISALLOW_COPY_AND_ASSIGN_(ValueHolderFactory);
};

class DefaultValueHolderFactory : public ValueHolderFactory {
public:
DefaultValueHolderFactory() {}
virtual ValueHolder* MakeNewHolder() const { return new ValueHolder(); }

private:
GTEST_DISALLOW_COPY_AND_ASSIGN_(DefaultValueHolderFactory);
};

class InstanceValueHolderFactory : public ValueHolderFactory {
public:
explicit InstanceValueHolderFactory(const T& value) : value_(value) {}
virtual ValueHolder* MakeNewHolder() const {
return new ValueHolder(value_);
}

private:
const T value_; // The value for each thread.

GTEST_DISALLOW_COPY_AND_ASSIGN_(InstanceValueHolderFactory);
};

scoped_ptr<ValueHolderFactory> default_factory_;

GTEST_DISALLOW_COPY_AND_ASSIGN_(ThreadLocal);
};
Expand Down Expand Up @@ -1993,10 +2027,11 @@ extern "C" inline void DeleteThreadLocalValue(void* value_holder) {
template <typename T>
class ThreadLocal {
public:
ThreadLocal() : key_(CreateKey()),
default_() {}
explicit ThreadLocal(const T& value) : key_(CreateKey()),
default_(value) {}
ThreadLocal()
: key_(CreateKey()), default_factory_(new DefaultValueHolderFactory()) {}
explicit ThreadLocal(const T& value)
: key_(CreateKey()),
default_factory_(new InstanceValueHolderFactory(value)) {}

~ThreadLocal() {
// Destroys the managed object for the current thread, if any.
Expand All @@ -2016,6 +2051,7 @@ class ThreadLocal {
// Holds a value of type T.
class ValueHolder : public ThreadLocalValueHolderBase {
public:
ValueHolder() : value_() {}
explicit ValueHolder(const T& value) : value_(value) {}

T* pointer() { return &value_; }
Expand All @@ -2041,15 +2077,47 @@ class ThreadLocal {
return CheckedDowncastToActualType<ValueHolder>(holder)->pointer();
}

ValueHolder* const new_holder = new ValueHolder(default_);
ValueHolder* const new_holder = default_factory_->MakeNewHolder();
ThreadLocalValueHolderBase* const holder_base = new_holder;
GTEST_CHECK_POSIX_SUCCESS_(pthread_setspecific(key_, holder_base));
return new_holder->pointer();
}

class ValueHolderFactory {
public:
ValueHolderFactory() {}
virtual ~ValueHolderFactory() {}
virtual ValueHolder* MakeNewHolder() const = 0;

private:
GTEST_DISALLOW_COPY_AND_ASSIGN_(ValueHolderFactory);
};

class DefaultValueHolderFactory : public ValueHolderFactory {
public:
DefaultValueHolderFactory() {}
virtual ValueHolder* MakeNewHolder() const { return new ValueHolder(); }

private:
GTEST_DISALLOW_COPY_AND_ASSIGN_(DefaultValueHolderFactory);
};

class InstanceValueHolderFactory : public ValueHolderFactory {
public:
explicit InstanceValueHolderFactory(const T& value) : value_(value) {}
virtual ValueHolder* MakeNewHolder() const {
return new ValueHolder(value_);
}

private:
const T value_; // The value for each thread.

GTEST_DISALLOW_COPY_AND_ASSIGN_(InstanceValueHolderFactory);
};

// A key pthreads uses for looking up per-thread values.
const pthread_key_t key_;
const T default_; // The default value for each thread.
scoped_ptr<ValueHolderFactory> default_factory_;

GTEST_DISALLOW_COPY_AND_ASSIGN_(ThreadLocal);
};
Expand Down
41 changes: 9 additions & 32 deletions test/gtest-port_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1149,13 +1149,6 @@ TEST(ThreadLocalTest, ParameterizedConstructorSetsDefault) {
EXPECT_STREQ("foo", result.c_str());
}

# if !GTEST_HAS_MUTEX_AND_THREAD_LOCAL_

// Tests in this section depend on that Google Test's own ThreadLocal
// implementation stores a copy of the default value shared by all
// threads. We don't want to test this for an external implementation received
// through GTEST_HAS_MUTEX_AND_THREAD_LOCAL_.

// Keeps track of whether of destructors being called on instances of
// DestructorTracker. On Windows, waits for the destructor call reports.
class DestructorCall {
Expand Down Expand Up @@ -1240,25 +1233,18 @@ TEST(ThreadLocalTest, DestroysManagedObjectForOwnThreadWhenDying) {
DestructorCall::ResetList();

{
// The next line default constructs a DestructorTracker object as
// the default value of objects managed by thread_local_tracker.
ThreadLocal<DestructorTracker> thread_local_tracker;
ASSERT_EQ(1U, DestructorCall::List().size());
ASSERT_FALSE(DestructorCall::List()[0]->CheckDestroyed());
ASSERT_EQ(0U, DestructorCall::List().size());

// This creates another DestructorTracker object for the main thread.
thread_local_tracker.get();
ASSERT_EQ(2U, DestructorCall::List().size());
ASSERT_EQ(1U, DestructorCall::List().size());
ASSERT_FALSE(DestructorCall::List()[0]->CheckDestroyed());
ASSERT_FALSE(DestructorCall::List()[1]->CheckDestroyed());
}

// Now thread_local_tracker has died. It should have destroyed both the
// default value shared by all threads and the value for the main
// thread.
ASSERT_EQ(2U, DestructorCall::List().size());
// Now thread_local_tracker has died.
ASSERT_EQ(1U, DestructorCall::List().size());
EXPECT_TRUE(DestructorCall::List()[0]->CheckDestroyed());
EXPECT_TRUE(DestructorCall::List()[1]->CheckDestroyed());

DestructorCall::ResetList();
}
Expand All @@ -1269,35 +1255,26 @@ TEST(ThreadLocalTest, DestroysManagedObjectAtThreadExit) {
DestructorCall::ResetList();

{
// The next line default constructs a DestructorTracker object as
// the default value of objects managed by thread_local_tracker.
ThreadLocal<DestructorTracker> thread_local_tracker;
ASSERT_EQ(1U, DestructorCall::List().size());
ASSERT_FALSE(DestructorCall::List()[0]->CheckDestroyed());
ASSERT_EQ(0U, DestructorCall::List().size());

// This creates another DestructorTracker object in the new thread.
ThreadWithParam<ThreadParam> thread(
&CallThreadLocalGet, &thread_local_tracker, NULL);
thread.Join();

// The thread has exited, and we should have another DestroyedTracker
// The thread has exited, and we should have a DestroyedTracker
// instance created for it. But it may not have been destroyed yet.
// The instance for the main thread should still persist.
ASSERT_EQ(2U, DestructorCall::List().size());
ASSERT_FALSE(DestructorCall::List()[0]->CheckDestroyed());
ASSERT_EQ(1U, DestructorCall::List().size());
}

// The thread has exited and thread_local_tracker has died. The default
// value should have been destroyed too.
ASSERT_EQ(2U, DestructorCall::List().size());
// The thread has exited and thread_local_tracker has died.
ASSERT_EQ(1U, DestructorCall::List().size());
EXPECT_TRUE(DestructorCall::List()[0]->CheckDestroyed());
EXPECT_TRUE(DestructorCall::List()[1]->CheckDestroyed());

DestructorCall::ResetList();
}

# endif // !GTEST_HAS_MUTEX_AND_THREAD_LOCAL_

TEST(ThreadLocalTest, ThreadLocalMutationsAffectOnlyCurrentThread) {
ThreadLocal<std::string> thread_local_string;
thread_local_string.set("Foo");
Expand Down

0 comments on commit 0afa989

Please sign in to comment.