Skip to content

Commit

Permalink
Export of internal Abseil changes
Browse files Browse the repository at this point in the history
--
dea3e4f33f16bdb1d89cad1f8055b81c0c0cb554 by Andy Getzendanner <durandal@google.com>:

Validate in log_severity_test that flags of type absl::LogSeverity are lock-free.

PiperOrigin-RevId: 293454285

--
2a0cd2d8dc193a0cbff4ffa6c5c7037745507419 by Derek Mauro <dmauro@google.com>:

Update the testing instructions in CONTRIBUTING.md

PiperOrigin-RevId: 293436013

--
cec91c3f635b0b4c8a60955e5926dba4ed980898 by Gennadiy Rozental <rogeeff@google.com>:

Introduce struct to represent storage for flag value and normalize naming of internal structs in Flag implementation.

There is no semantic changes in this CL. All the internal structs are now named as Flag... We also stop using flags_internal:: qualifications for most of them since the names are unique enough by themselves.

PiperOrigin-RevId: 293251467
GitOrigin-RevId: dea3e4f33f16bdb1d89cad1f8055b81c0c0cb554
Change-Id: I161aecc9509edae3e4b77eead02df684b2ce7087
  • Loading branch information
Abseil Team authored and suertreus committed Feb 5, 2020
1 parent 08a7e7b commit 72382c2
Show file tree
Hide file tree
Showing 9 changed files with 245 additions and 210 deletions.
9 changes: 6 additions & 3 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -123,10 +123,13 @@ will be expected to conform to the style outlined

## Running Tests

Use "bazel test <>" functionality to run the unit tests.
If you have [Bazel](https://bazel.build/) installed, use `bazel test
--test_tag_filters="-benchmark" ...` to run the unit tests.

Prerequisites for building and running tests are listed in
[README.md](README.md)
If you are running the Linux operating system and have
[Docker](https://www.docker.com/) installed, you can also run the `linux_*.sh`
scripts under the `ci/`(https://github.com/abseil/abseil-cpp/tree/master/ci)
directory to test Abseil under a variety of conditions.

## Abseil Committers

Expand Down
1 change: 1 addition & 0 deletions absl/base/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -675,6 +675,7 @@ cc_test(
linkopts = ABSL_DEFAULT_LINKOPTS,
deps = [
":log_severity",
"//absl/flags:flag_internal",
"//absl/flags:marshalling",
"//absl/strings",
"@com_google_googletest//:gtest_main",
Expand Down
1 change: 1 addition & 0 deletions absl/base/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -610,6 +610,7 @@ absl_cc_test(
SRCS
"log_severity_test.cc"
DEPS
absl::flags_internal
absl::flags_marshalling
absl::log_severity
absl::strings
Expand Down
5 changes: 5 additions & 0 deletions absl/base/log_severity_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@

#include "gmock/gmock.h"
#include "gtest/gtest.h"
#include "absl/flags/internal/flag.h"
#include "absl/flags/marshalling.h"
#include "absl/strings/str_cat.h"

Expand Down Expand Up @@ -51,6 +52,10 @@ TEST(StreamTest, Works) {
Eq("absl::LogSeverity(4)"));
}

static_assert(
absl::flags_internal::IsAtomicFlagTypeTrait<absl::LogSeverity>::value,
"Flags of type absl::LogSeverity ought to be lock-free.");

using ParseFlagFromOutOfRangeIntegerTest = TestWithParam<int64_t>;
INSTANTIATE_TEST_SUITE_P(
Instantiation, ParseFlagFromOutOfRangeIntegerTest,
Expand Down
2 changes: 1 addition & 1 deletion absl/flags/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ cc_library(
],
copts = ABSL_DEFAULT_COPTS,
linkopts = ABSL_DEFAULT_LINKOPTS,
visibility = ["//visibility:private"],
visibility = ["//absl/base:__subpackages__"],
deps = [
":config",
":handle",
Expand Down
12 changes: 6 additions & 6 deletions absl/flags/flag.h
Original file line number Diff line number Diff line change
Expand Up @@ -120,11 +120,11 @@ class Flag {
return impl_;
}

impl_ = new flags_internal::Flag<T>(
name_, filename_, marshalling_op_,
{flags_internal::FlagHelpSrc(help_gen_),
flags_internal::FlagHelpSrcKind::kGenFunc},
default_value_gen_);
impl_ =
new flags_internal::Flag<T>(name_, filename_, marshalling_op_,
{flags_internal::FlagHelpMsg(help_gen_),
flags_internal::FlagHelpKind::kGenFunc},
default_value_gen_);
inited_.store(true, std::memory_order_release);
}

Expand Down Expand Up @@ -152,7 +152,7 @@ class Flag {
T Get() const { return GetImpl()->Get(); }
bool AtomicGet(T* v) const { return GetImpl()->AtomicGet(v); }
void Set(const T& v) { GetImpl()->Set(v); }
void SetCallback(const flags_internal::FlagCallback mutation_callback) {
void SetCallback(const flags_internal::FlagCallbackFunc mutation_callback) {
GetImpl()->SetCallback(mutation_callback);
}
void InvokeCallback() { GetImpl()->InvokeCallback(); }
Expand Down
6 changes: 3 additions & 3 deletions absl/flags/flag_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,8 @@ void TestCallback() {}

template <typename T>
bool TestConstructionFor() {
constexpr flags::HelpInitArg help_arg{flags::FlagHelpSrc("literal help"),
flags::FlagHelpSrcKind::kLiteral};
constexpr flags::FlagHelpArg help_arg{flags::FlagHelpMsg("literal help"),
flags::FlagHelpKind::kLiteral};
constexpr flags::Flag<T> f1("f1", "file", &flags::FlagMarshallingOps<T>,
help_arg, &TestMakeDflt<T>);
EXPECT_EQ(f1.Name(), "f1");
Expand All @@ -61,7 +61,7 @@ bool TestConstructionFor() {

ABSL_CONST_INIT static flags::Flag<T> f2(
"f2", "file", &flags::FlagMarshallingOps<T>,
{flags::FlagHelpSrc(&TestHelpMsg), flags::FlagHelpSrcKind::kGenFunc},
{flags::FlagHelpMsg(&TestHelpMsg), flags::FlagHelpKind::kGenFunc},
&TestMakeDflt<T>);
flags::FlagRegistrar<T, false>(&f2).OnUpdate(TestCallback);

Expand Down
47 changes: 23 additions & 24 deletions absl/flags/internal/flag.cc
Original file line number Diff line number Diff line change
Expand Up @@ -91,11 +91,11 @@ void FlagImpl::Init() {

absl::MutexLock lock(reinterpret_cast<absl::Mutex*>(&data_guard_));

if (cur_ != nullptr) {
if (value_.dynamic != nullptr) {
inited_.store(true, std::memory_order_release);
} else {
// Need to initialize cur field.
cur_ = MakeInitValue().release();
value_.dynamic = MakeInitValue().release();
StoreAtomic();
inited_.store(true, std::memory_order_release);
}
Expand All @@ -117,15 +117,15 @@ void FlagImpl::Destroy() {
absl::MutexLock l(DataGuard());

// Values are heap allocated for Abseil Flags.
if (cur_) Delete(op_, cur_);
if (value_.dynamic) Delete(op_, value_.dynamic);

// Release the dynamically allocated default value if any.
if (def_kind_ == FlagDefaultSrcKind::kDynamicValue) {
Delete(op_, default_src_.dynamic_value);
}

// If this flag has an assigned callback, release callback data.
if (callback_data_) delete callback_data_;
if (callback_) delete callback_;
}

absl::MutexLock l(&flag_mutex_lifetime_guard);
Expand All @@ -150,8 +150,8 @@ std::string FlagImpl::Filename() const {
}

std::string FlagImpl::Help() const {
return help_source_kind_ == FlagHelpSrcKind::kLiteral ? help_.literal
: help_.gen_func();
return help_source_kind_ == FlagHelpKind::kLiteral ? help_.literal
: help_.gen_func();
}

bool FlagImpl::IsModified() const {
Expand All @@ -174,27 +174,26 @@ std::string FlagImpl::DefaultValue() const {
std::string FlagImpl::CurrentValue() const {
absl::MutexLock l(DataGuard());

return Unparse(marshalling_op_, cur_);
return Unparse(marshalling_op_, value_.dynamic);
}

void FlagImpl::SetCallback(
const flags_internal::FlagCallback mutation_callback) {
void FlagImpl::SetCallback(const FlagCallbackFunc mutation_callback) {
absl::MutexLock l(DataGuard());

if (callback_data_ == nullptr) {
callback_data_ = new CallbackData;
if (callback_ == nullptr) {
callback_ = new FlagCallback;
}
callback_data_->func = mutation_callback;
callback_->func = mutation_callback;

InvokeCallback();
}

void FlagImpl::InvokeCallback() const {
if (!callback_data_) return;
if (!callback_) return;

// Make a copy of the C-style function pointer that we are about to invoke
// before we release the lock guarding it.
FlagCallback cb = callback_data_->func;
FlagCallbackFunc cb = callback_->func;

// If the flag has a mutation callback this function invokes it. While the
// callback is being invoked the primary flag's mutex is unlocked and it is
Expand All @@ -208,7 +207,7 @@ void FlagImpl::InvokeCallback() const {
// completed. Requires that *primary_lock be held in exclusive mode; it may be
// released and reacquired by the implementation.
MutexRelock relock(DataGuard());
absl::MutexLock lock(&callback_data_->guard);
absl::MutexLock lock(&callback_->guard);
cb();
}

Expand Down Expand Up @@ -267,22 +266,22 @@ void FlagImpl::Read(void* dst, const flags_internal::FlagOpFn dst_op) const {
absl::StrCat("Flag '", Name(),
"' is defined as one type and declared as another"));
}
CopyConstruct(op_, cur_, dst);
CopyConstruct(op_, value_.dynamic, dst);
}

void FlagImpl::StoreAtomic() {
size_t data_size = Sizeof(op_);

if (data_size <= sizeof(int64_t)) {
int64_t t = 0;
std::memcpy(&t, cur_, data_size);
atomics_.small_atomic.store(t, std::memory_order_release);
std::memcpy(&t, value_.dynamic, data_size);
value_.atomics.small_atomic.store(t, std::memory_order_release);
}
#if defined(ABSL_FLAGS_INTERNAL_ATOMIC_DOUBLE_WORD)
else if (data_size <= sizeof(FlagsInternalTwoWordsType)) {
FlagsInternalTwoWordsType t{0, 0};
std::memcpy(&t, cur_, data_size);
atomics_.big_atomic.store(t, std::memory_order_release);
std::memcpy(&t, value_.dynamic, data_size);
value_.atomics.big_atomic.store(t, std::memory_order_release);
}
#endif
}
Expand Down Expand Up @@ -313,7 +312,7 @@ void FlagImpl::Write(const void* src, const flags_internal::FlagOpFn src_op) {

modified_ = true;
counter_++;
Copy(op_, src, cur_);
Copy(op_, src, value_.dynamic);

StoreAtomic();
InvokeCallback();
Expand All @@ -334,7 +333,7 @@ bool FlagImpl::SetFromString(absl::string_view value, FlagSettingMode set_mode,
switch (set_mode) {
case SET_FLAGS_VALUE: {
// set or modify the flag's value
if (!TryParse(&cur_, value, err)) return false;
if (!TryParse(&value_.dynamic, value, err)) return false;
modified_ = true;
counter_++;
StoreAtomic();
Expand All @@ -348,7 +347,7 @@ bool FlagImpl::SetFromString(absl::string_view value, FlagSettingMode set_mode,
case SET_FLAG_IF_DEFAULT: {
// set the flag's value, but only if it hasn't been set by someone else
if (!modified_) {
if (!TryParse(&cur_, value, err)) return false;
if (!TryParse(&value_.dynamic, value, err)) return false;
modified_ = true;
counter_++;
StoreAtomic();
Expand Down Expand Up @@ -381,7 +380,7 @@ bool FlagImpl::SetFromString(absl::string_view value, FlagSettingMode set_mode,

if (!modified_) {
// Need to set both default value *and* current, in this case
Copy(op_, default_src_.dynamic_value, cur_);
Copy(op_, default_src_.dynamic_value, value_.dynamic);
StoreAtomic();
InvokeCallback();
}
Expand Down
Loading

0 comments on commit 72382c2

Please sign in to comment.