Skip to content

Commit

Permalink
Refactor sizeGivenName(), sizeGivenStatsOptions() into structSize(), …
Browse files Browse the repository at this point in the history
…structSizeWithOptions()

Signed-off-by: James Buckland <jbuckland@google.com>
  • Loading branch information
ambuc committed Jul 12, 2018
1 parent 9a30244 commit aeb9251
Show file tree
Hide file tree
Showing 7 changed files with 17 additions and 18 deletions.
2 changes: 1 addition & 1 deletion source/common/common/block_memory_hash_set.h
Original file line number Diff line number Diff line change
Expand Up @@ -330,7 +330,7 @@ template <class Value> class BlockMemoryHashSet : public Logger::Loggable<Logger
// sizeof(Cell) includes 'sizeof Value' which may not be accurate. So we need to
// subtract that off, and add the template method's view of the actual value-size.
uint64_t cell_size =
align(sizeof(Cell) + Value::sizeGivenStatsOptions(stats_options) - sizeof(Value));
align(sizeof(Cell) + Value::structSizeWithOptions(stats_options) - sizeof(Value));
return cell_index * cell_size;
}

Expand Down
12 changes: 6 additions & 6 deletions source/common/stats/stats_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -40,12 +40,12 @@ bool regexStartsWithDot(absl::string_view regex) {
// Normally the compiler would do this, but because name_ is a flexible-array-length
// element, the compiler can't. RawStatData is put into an array in HotRestartImpl, so
// it's important that each element starts on the required alignment for the type.
uint64_t RawStatData::sizeGivenName(const absl::string_view name) {
return roundUpMultipleNaturalAlignment(sizeof(RawStatData) + name.size() + 1);
uint64_t RawStatData::structSize(uint64_t name_size) {
return roundUpMultipleNaturalAlignment(sizeof(RawStatData) + name_size + 1);
}

uint64_t RawStatData::sizeGivenStatsOptions(const StatsOptions& stats_options) {
return roundUpMultipleNaturalAlignment(sizeof(RawStatData) + stats_options.maxNameLength() + 1);
uint64_t RawStatData::structSizeWithOptions(const StatsOptions& stats_options) {
return structSize(stats_options.maxNameLength());
}

std::string Utility::sanitizeStatsName(const std::string& name) {
Expand Down Expand Up @@ -136,7 +136,7 @@ bool TagExtractorImpl::extractTag(const std::string& stat_name, std::vector<Tag>
}

RawStatData* HeapRawStatDataAllocator::alloc(const std::string& name) {
uint64_t num_bytes_to_allocate = RawStatData::sizeGivenName(name);
uint64_t num_bytes_to_allocate = RawStatData::structSize(name.size());
RawStatData* data = static_cast<RawStatData*>(::calloc(num_bytes_to_allocate, 1));
if (data == nullptr) {
throw EnvoyException("HeapRawStatDataAllocator: unable to allocate a new stat");
Expand Down Expand Up @@ -344,7 +344,7 @@ void RawStatData::initialize(absl::string_view key, uint64_t xfer_size) {

void RawStatData::checkAndInit(absl::string_view key, uint64_t num_bytes_allocated) {
uint64_t xfer_size = key.size();
ASSERT(sizeof(RawStatData) + xfer_size + 1 <= num_bytes_allocated);
ASSERT(structSize(xfer_size) <= num_bytes_allocated);

initialize(key, xfer_size);
}
Expand Down
11 changes: 5 additions & 6 deletions source/common/stats/stats_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ class Utility {
* it can be allocated from shared memory if needed.
*
* @note Due to name_ being variable size, sizeof(RawStatData) probably isn't useful. Use
* RawStatData::sizeGivenName() or RawStatData::sizeGivenStatsOptions() instead.
* RawStatData::structSize() or RawStatData::structSizeWithOptions() instead.
*/
struct RawStatData {

Expand All @@ -207,14 +207,13 @@ struct RawStatData {
* and padding for alignment. Required for the HeapRawStatDataAllocator, which does not truncate
* at a maximum stat name length.
*/
static uint64_t sizeGivenName(const absl::string_view name);
static uint64_t structSize(uint64_t name_size);

/**
* Returns the size of this struct, accounting for the length of name_
* and padding for alignment. Required by BlockMemoryHashSet, which has the context to report
* StatsOptions.
* Wrapper for structSize, taking a StatsOptions struct.
* Required by BlockMemoryHashSet, which has the context to supply StatsOptions.
*/
static uint64_t sizeGivenStatsOptions(const StatsOptions& stats_options);
static uint64_t structSizeWithOptions(const StatsOptions& stats_options);

/**
* Initializes this object to have the specified key,
Expand Down
4 changes: 2 additions & 2 deletions source/server/hot_restart_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ static BlockMemoryHashSetOptions blockMemHashOptions(uint64_t max_stats) {
SharedMemory& SharedMemory::initialize(uint64_t stats_set_size, Options& options) {
Api::OsSysCalls& os_sys_calls = Api::OsSysCallsSingleton::get();

const uint64_t entry_size = Stats::RawStatData::sizeGivenStatsOptions(options.statsOptions());
const uint64_t entry_size = Stats::RawStatData::structSizeWithOptions(options.statsOptions());
const uint64_t total_size = sizeof(SharedMemory) + stats_set_size;

int flags = O_RDWR;
Expand Down Expand Up @@ -172,7 +172,7 @@ void HotRestartImpl::free(Stats::RawStatData& data) {
bool key_removed = stats_set_->remove(data.key());
ASSERT(key_removed);
memset(static_cast<void*>(&data), 0,
Stats::RawStatData::sizeGivenStatsOptions(options_.statsOptions()));
Stats::RawStatData::structSizeWithOptions(options_.statsOptions()));
}

int HotRestartImpl::bindDomainSocket(uint64_t id) {
Expand Down
2 changes: 1 addition & 1 deletion test/common/common/block_memory_hash_set_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ class BlockMemoryHashSetTest : public testing::Test {
memcpy(name, key.data(), xfer);
name[xfer] = '\0';
}
static uint64_t sizeGivenStatsOptions(const Stats::StatsOptions& stats_options) {
static uint64_t structSizeWithOptions(const Stats::StatsOptions& stats_options) {
UNREFERENCED_PARAMETER(stats_options);
return sizeof(TestValue);
}
Expand Down
2 changes: 1 addition & 1 deletion test/common/stats/stats_impl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -528,7 +528,7 @@ TEST(RawStatDataTest, Truncate) {
Stats::StatsOptionsImpl stats_options;
const std::string long_string(stats_options.maxNameLength() + 1, 'A');
RawStatData* stat =
static_cast<RawStatData*>(::calloc(RawStatData::sizeGivenStatsOptions(stats_options), 1));
static_cast<RawStatData*>(::calloc(RawStatData::structSizeWithOptions(stats_options), 1));
EXPECT_LOG_CONTAINS("warning", "is too long with",
stat->truncateAndInit(long_string, stats_options));
::free(stat);
Expand Down
2 changes: 1 addition & 1 deletion test/test_common/utility.h
Original file line number Diff line number Diff line change
Expand Up @@ -347,7 +347,7 @@ class TestAllocator : public RawStatDataAllocator {
CSmartPtr<RawStatData, freeAdapter>& stat_ref = stats_[name];
if (!stat_ref) {
stat_ref.reset(static_cast<RawStatData*>(
::calloc(RawStatData::sizeGivenStatsOptions(stats_options), 1)));
::calloc(RawStatData::structSizeWithOptions(stats_options), 1)));
stat_ref->truncateAndInit(name, stats_options);
} else {
stat_ref->ref_count_++;
Expand Down

0 comments on commit aeb9251

Please sign in to comment.