Skip to content

Commit

Permalink
Minor fixes to comply with static code analysis checks
Browse files Browse the repository at this point in the history
  • Loading branch information
r1viollet committed Oct 9, 2023
1 parent 5a59702 commit aa4e705
Show file tree
Hide file tree
Showing 5 changed files with 21 additions and 19 deletions.
2 changes: 1 addition & 1 deletion include/lib/allocation_tracker.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ class AllocationTracker {
static TrackerThreadLocalState *get_tl_state();

private:
using AdressSet = std::unordered_set<uintptr_t>;
static constexpr unsigned k_ratio_max_elt_to_bitset_size = 16;

struct TrackerState {
void init(bool track_alloc, bool track_dealloc) {
Expand Down
9 changes: 5 additions & 4 deletions src/ddprof_worker.cc
Original file line number Diff line number Diff line change
Expand Up @@ -118,8 +118,9 @@ DDRes worker_update_stats(DDProfWorkerContext &worker_context,
ddprof_stats_set(STATS_DSO_NEW_DSO,
dso_hdr._stats.sum_event_metric(DsoStats::kNewDso));
ddprof_stats_set(STATS_DSO_SIZE, dso_hdr.get_nb_dso());
ddprof_stats_set(STATS_UNMATCHED_DEALLOCATION_COUNT,
worker_context.live_allocation.get_nb_unmatched_deallocations());
ddprof_stats_set(
STATS_UNMATCHED_DEALLOCATION_COUNT,
worker_context.live_allocation.get_nb_unmatched_deallocations());
// Symbol stats
DDRES_CHECK_FWD(symbols_update_stats(us.symbol_hdr));

Expand Down Expand Up @@ -387,8 +388,8 @@ DDRes ddprof_pr_sample(DDProfContext &ctx, perf_event_sample *sample,
// Aggregate if unwinding went well (todo : fatal error propagation)
if (!IsDDResFatal(res)) {
struct UnwindState *us = ctx.worker_ctx.us;
if (Any(EventAggregationMode::kLiveSum & watcher->aggregation_mode)
&& sample->addr) {
if (Any(EventAggregationMode::kLiveSum & watcher->aggregation_mode) &&
sample->addr) {
// null address means we should not account it
ctx.worker_ctx.live_allocation.register_allocation(
us->output, sample->addr, sample->period, watcher_pos, sample->pid);
Expand Down
20 changes: 10 additions & 10 deletions src/lib/address_bitset.cc
Original file line number Diff line number Diff line change
Expand Up @@ -72,13 +72,13 @@ void AddressBitset::init(unsigned bitset_size) {
}

bool AddressBitset::add(uintptr_t addr) {
uint32_t significant_bits = hash_significant_bits(addr);
const uint32_t significant_bits = hash_significant_bits(addr);
// As per nsavoire's comment, it is better to use separate operators
// than to use the div instruction which generates an extra function call
// Also, the usage of a power of two value allows for bit operations
unsigned index_array = significant_bits / _nb_bits_per_word;
unsigned bit_offset = significant_bits % _nb_bits_per_word;
Word_t bit_in_element = (1UL << bit_offset);
const unsigned index_array = significant_bits / _nb_bits_per_word;
const unsigned bit_offset = significant_bits % _nb_bits_per_word;
const Word_t bit_in_element = (1UL << bit_offset);
// there is a possible race between checking the value
// and setting it
if (!(_address_bitset[index_array].fetch_or(bit_in_element) &
Expand All @@ -92,10 +92,10 @@ bool AddressBitset::add(uintptr_t addr) {
}

bool AddressBitset::remove(uintptr_t addr) {
int significant_bits = hash_significant_bits(addr);
unsigned index_array = significant_bits / _nb_bits_per_word;
unsigned bit_offset = significant_bits % _nb_bits_per_word;
Word_t bit_in_element = (1UL << bit_offset);
const int significant_bits = hash_significant_bits(addr);
const unsigned index_array = significant_bits / _nb_bits_per_word;
const unsigned bit_offset = significant_bits % _nb_bits_per_word;
const Word_t bit_in_element = (1UL << bit_offset);
if (_address_bitset[index_array].fetch_and(~bit_in_element) &
bit_in_element) {
_nb_addresses.fetch_sub(1, std::memory_order_relaxed);
Expand All @@ -108,9 +108,9 @@ bool AddressBitset::remove(uintptr_t addr) {

void AddressBitset::clear() {
for (unsigned i = 0; i < _k_nb_words; ++i) {
Word_t original_value = _address_bitset[i].exchange(0);
const Word_t original_value = _address_bitset[i].exchange(0);
// Count number of set bits in original_value
int num_set_bits = std::popcount(original_value);
const int num_set_bits = std::popcount(original_value);
if (num_set_bits > 0) {
_nb_addresses.fetch_sub(num_set_bits, std::memory_order_relaxed);
}
Expand Down
7 changes: 4 additions & 3 deletions src/lib/allocation_tracker.cc
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,8 @@ DDRes AllocationTracker::init(uint64_t mem_profile_interval,
}
if (track_deallocations) {
// 16 times as we want to probability of collision to be low enough
_allocated_address_set = AddressBitset(liveallocation::kMaxTracked * 16);
_allocated_address_set = AddressBitset(liveallocation::kMaxTracked *
k_ratio_max_elt_to_bitset_size);
}
return ddprof::ring_buffer_attach(ring_buffer, &_pevent);
}
Expand All @@ -169,7 +170,8 @@ void AllocationTracker::allocation_tracking_free() {
}
TrackerThreadLocalState *tl_state = get_tl_state();
if (unlikely(!tl_state)) {
LOG_ONCE("Error: Unable to find tl_state during %s\n", __FUNCTION__);
const char *func_name = __FUNCTION__;
LOG_ONCE("Error: Unable to find tl_state during %s\n", func_name);
instance->free();
return;
}
Expand Down Expand Up @@ -264,7 +266,6 @@ void AllocationTracker::track_deallocation(uintptr_t addr,
// Reentrancy should be prevented by caller (by using ReentryGuard on
// TrackerThreadLocalState::reentry_guard).


if (!_state.track_deallocations || !_allocated_address_set.remove(addr)) {
return;
}
Expand Down
2 changes: 1 addition & 1 deletion tools/style-check.sh
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ SCRIPT_DIR=$( cd -- "$( dirname -- "${BASH_SOURCE[0]}" )" &> /dev/null && pwd )
cd "$SCRIPT_DIR/.."

# Find most recent clang-format, defaulting to an unqualified default
CLANG_FORMAT=$(command -v clang-format{-16,-15,-13,-12,-11,-10,-9,} | head -n1)
CLANG_FORMAT=$(command -v clang-format{-17,-16,-15,-13,-12,-11,-10,-9,} | head -n1)
if [ -z "${CLANG_FORMAT}" ]; then
echo "Please install clang-format"
exit 1
Expand Down

0 comments on commit aa4e705

Please sign in to comment.