From 9511d3cd599d95101cfca6c856a4df70ab92cbf1 Mon Sep 17 00:00:00 2001 From: Radoslaw Zarzynski Date: Thu, 6 Dec 2018 18:44:33 +0100 Subject: [PATCH] common: drop allocation tracking from bufferlist. Signed-off-by: Radoslaw Zarzynski --- src/common/buffer.cc | 77 +++++---------------------------- src/include/buffer.h | 12 ----- src/mds/MDCache.cc | 1 - src/mds/MDSRank.cc | 2 - src/mds/MDSRank.h | 1 - src/osd/OSD.cc | 6 --- src/osd/OSD.h | 3 -- src/test/CMakeLists.txt | 1 - src/test/bufferlist.cc | 89 +------------------------------------- src/unittest_bufferlist.sh | 19 -------- 10 files changed, 12 insertions(+), 199 deletions(-) delete mode 100755 src/unittest_bufferlist.sh diff --git a/src/common/buffer.cc b/src/common/buffer.cc index b1c533ed310..49f95cf85fe 100644 --- a/src/common/buffer.cc +++ b/src/common/buffer.cc @@ -52,41 +52,6 @@ using namespace ceph; # define bendl std::endl; } #endif - static std::atomic buffer_total_alloc { 0 }; - static std::atomic buffer_history_alloc_bytes { 0 }; - static std::atomic buffer_history_alloc_num { 0 }; - - const bool buffer_track_alloc = get_env_bool("CEPH_BUFFER_TRACK"); - - namespace { - void inc_total_alloc(unsigned len) { - if (buffer_track_alloc) - buffer_total_alloc += len; - } - - void dec_total_alloc(unsigned len) { - if (buffer_track_alloc) - buffer_total_alloc -= len; - } - - void inc_history_alloc(uint64_t len) { - if (buffer_track_alloc) { - buffer_history_alloc_bytes += len; - buffer_history_alloc_num++; - } - } - } // namespace - - int buffer::get_total_alloc() { - return buffer_total_alloc; - } - uint64_t buffer::get_history_alloc_bytes() { - return buffer_history_alloc_bytes; - } - uint64_t buffer::get_history_alloc_num() { - return buffer_history_alloc_num; - } - static std::atomic buffer_cached_crc { 0 }; static std::atomic buffer_cached_crc_adjusted { 0 }; static std::atomic buffer_missed_crc { 0 }; @@ -180,11 +145,6 @@ using namespace ceph; int mempool) : raw(dataptr, l, mempool), alignment(align) { - inc_total_alloc(len); - inc_history_alloc(len); - } - ~raw_combined() override { - dec_total_alloc(len); } raw* clone_empty() override { return create(len, alignment); @@ -233,18 +193,14 @@ using namespace ceph; } else { data = 0; } - inc_total_alloc(len); - inc_history_alloc(len); - bdout << "raw_malloc " << this << " alloc " << (void *)data << " " << l << " " << buffer::get_total_alloc() << bendl; + bdout << "raw_malloc " << this << " alloc " << (void *)data << " " << l << bendl; } raw_malloc(unsigned l, char *b) : raw(b, l) { - inc_total_alloc(len); - bdout << "raw_malloc " << this << " alloc " << (void *)data << " " << l << " " << buffer::get_total_alloc() << bendl; + bdout << "raw_malloc " << this << " alloc " << (void *)data << " " << l << bendl; } ~raw_malloc() override { free(data); - dec_total_alloc(len); - bdout << "raw_malloc " << this << " free " << (void *)data << " " << buffer::get_total_alloc() << bendl; + bdout << "raw_malloc " << this << " free " << (void *)data << " " << bendl; } raw* clone_empty() override { return new raw_malloc(len); @@ -269,14 +225,12 @@ using namespace ceph; #endif /* DARWIN */ if (!data) throw bad_alloc(); - inc_total_alloc(len); - inc_history_alloc(len); - bdout << "raw_posix_aligned " << this << " alloc " << (void *)data << " l=" << l << ", align=" << align << " total_alloc=" << buffer::get_total_alloc() << bendl; + bdout << "raw_posix_aligned " << this << " alloc " << (void *)data + << " l=" << l << ", align=" << align << bendl; } ~raw_posix_aligned() override { ::free(data); - dec_total_alloc(len); - bdout << "raw_posix_aligned " << this << " free " << (void *)data << " " << buffer::get_total_alloc() << bendl; + bdout << "raw_posix_aligned " << this << " free " << (void *)data << bendl; } raw* clone_empty() override { return new raw_posix_aligned(len, align); @@ -297,8 +251,6 @@ using namespace ceph; data = realdata + align - off; else data = realdata; - inc_total_alloc(len+align-1); - inc_history_alloc(len+align-1); //cout << "hack aligned " << (unsigned)data //<< " in raw " << (unsigned)realdata //<< " off " << off << std::endl; @@ -306,7 +258,6 @@ using namespace ceph; } ~raw_hack_aligned() { delete[] realdata; - dec_total_alloc(len+align-1); } raw* clone_empty() { return new raw_hack_aligned(len, align); @@ -326,18 +277,14 @@ using namespace ceph; data = new char[len]; else data = 0; - inc_total_alloc(len); - inc_history_alloc(len); - bdout << "raw_char " << this << " alloc " << (void *)data << " " << l << " " << buffer::get_total_alloc() << bendl; + bdout << "raw_char " << this << " alloc " << (void *)data << " " << l << bendl; } raw_char(unsigned l, char *b) : raw(b, l) { - inc_total_alloc(len); - bdout << "raw_char " << this << " alloc " << (void *)data << " " << l << " " << buffer::get_total_alloc() << bendl; + bdout << "raw_char " << this << " alloc " << (void *)data << " " << l << bendl; } ~raw_char() override { delete[] data; - dec_total_alloc(len); - bdout << "raw_char " << this << " free " << (void *)data << " " << buffer::get_total_alloc() << bendl; + bdout << "raw_char " << this << " free " << (void *)data << bendl; } raw* clone_empty() override { return new raw_char(len); @@ -349,14 +296,12 @@ using namespace ceph; MEMPOOL_CLASS_HELPERS(); explicit raw_claimed_char(unsigned l, char *b) : raw(b, l) { - inc_total_alloc(len); bdout << "raw_claimed_char " << this << " alloc " << (void *)data - << " " << l << " " << buffer::get_total_alloc() << bendl; + << " " << l << bendl; } ~raw_claimed_char() override { - dec_total_alloc(len); bdout << "raw_claimed_char " << this << " free " << (void *)data - << " " << buffer::get_total_alloc() << bendl; + << bendl; } raw* clone_empty() override { return new raw_char(len); diff --git a/src/include/buffer.h b/src/include/buffer.h index 56c5e8154e2..80b50d0cfd5 100644 --- a/src/include/buffer.h +++ b/src/include/buffer.h @@ -113,18 +113,6 @@ namespace buffer CEPH_BUFFER_API { }; - /// total bytes allocated - int get_total_alloc(); - - /// history total bytes allocated - uint64_t get_history_alloc_bytes(); - - /// total num allocated - uint64_t get_history_alloc_num(); - - /// enable/disable alloc tracking - void track_alloc(bool b); - /// count of cached crc hits (matching input) int get_cached_crc(); /// count of cached crc hits (mismatching input, required adjustment) diff --git a/src/mds/MDCache.cc b/src/mds/MDCache.cc index a83c52142e3..7fe58874517 100644 --- a/src/mds/MDCache.cc +++ b/src/mds/MDCache.cc @@ -7530,7 +7530,6 @@ void MDCache::check_memory_usage() << ", rss " << last.get_rss() << ", heap " << last.get_heap() << ", baseline " << baseline.get_heap() - << ", buffers " << (buffer::get_total_alloc() >> 10) << ", " << num_inodes_with_caps << " / " << CInode::count() << " inodes have caps" << ", " << Capability::count() << " caps, " << caps_per_inode << " caps per inode" << dendl; diff --git a/src/mds/MDSRank.cc b/src/mds/MDSRank.cc index faa1a92464d..6290ca90744 100644 --- a/src/mds/MDSRank.cc +++ b/src/mds/MDSRank.cc @@ -1103,7 +1103,6 @@ void MDSRank::update_mlogger() mlogger->set(l_mdm_dns, CDentry::decrements()); mlogger->set(l_mdm_capa, Capability::increments()); mlogger->set(l_mdm_caps, Capability::decrements()); - mlogger->set(l_mdm_buf, buffer::get_total_alloc()); } } @@ -3109,7 +3108,6 @@ void MDSRank::create_logger() mdm_plb.add_u64_counter(l_mdm_capa, "cap+", "Capabilities added"); mdm_plb.add_u64_counter(l_mdm_caps, "cap-", "Capabilities removed"); mdm_plb.add_u64(l_mdm_heap, "heap", "Heap size"); - mdm_plb.add_u64(l_mdm_buf, "buf", "Buffer size"); mdm_plb.set_prio_default(PerfCountersBuilder::PRIO_DEBUGONLY); mdm_plb.add_u64(l_mdm_rss, "rss", "RSS"); diff --git a/src/mds/MDSRank.h b/src/mds/MDSRank.h index 10a533a1e59..19044e26d50 100644 --- a/src/mds/MDSRank.h +++ b/src/mds/MDSRank.h @@ -97,7 +97,6 @@ enum { l_mdm_caps, l_mdm_rss, l_mdm_heap, - l_mdm_buf, l_mdm_last, }; diff --git a/src/osd/OSD.cc b/src/osd/OSD.cc index 06f945ecbe5..b260d0d4000 100644 --- a/src/osd/OSD.cc +++ b/src/osd/OSD.cc @@ -3336,9 +3336,6 @@ void OSD::create_logger() "rbt", PerfCountersBuilder::PRIO_INTERESTING); osd_plb.add_u64(l_osd_loadavg, "loadavg", "CPU load"); - osd_plb.add_u64(l_osd_buf, "buffer_bytes", "Total allocated buffer size", NULL, 0, unit_t(UNIT_BYTES)); - osd_plb.add_u64(l_osd_history_alloc_bytes, "history_alloc_Mbytes", NULL, 0, unit_t(UNIT_BYTES)); - osd_plb.add_u64(l_osd_history_alloc_num, "history_alloc_num"); osd_plb.add_u64( l_osd_cached_crc, "cached_crc", "Total number getting crc from crc_cache"); osd_plb.add_u64( @@ -5059,9 +5056,6 @@ void OSD::tick_without_osd_lock() ceph_assert(tick_timer_lock.is_locked()); dout(10) << "tick_without_osd_lock" << dendl; - logger->set(l_osd_buf, buffer::get_total_alloc()); - logger->set(l_osd_history_alloc_bytes, shift_round_up(buffer::get_history_alloc_bytes(), 20)); - logger->set(l_osd_history_alloc_num, buffer::get_history_alloc_num()); logger->set(l_osd_cached_crc, buffer::get_cached_crc()); logger->set(l_osd_cached_crc_adjusted, buffer::get_cached_crc_adjusted()); logger->set(l_osd_missed_crc, buffer::get_missed_crc()); diff --git a/src/osd/OSD.h b/src/osd/OSD.h index 7253e9aa14a..246576e8400 100644 --- a/src/osd/OSD.h +++ b/src/osd/OSD.h @@ -124,9 +124,6 @@ enum { l_osd_rbytes, l_osd_loadavg, - l_osd_buf, - l_osd_history_alloc_bytes, - l_osd_history_alloc_num, l_osd_cached_crc, l_osd_cached_crc_adjusted, l_osd_missed_crc, diff --git a/src/test/CMakeLists.txt b/src/test/CMakeLists.txt index ec250fc7214..5876bd8d610 100644 --- a/src/test/CMakeLists.txt +++ b/src/test/CMakeLists.txt @@ -534,7 +534,6 @@ add_ceph_test(test_objectstore_memstore.sh ${CMAKE_CURRENT_SOURCE_DIR}/test_obje #add_ceph_test(test_pidfile.sh ${CMAKE_CURRENT_SOURCE_DIR}/test_pidfile.sh) add_ceph_test(smoke.sh ${CMAKE_CURRENT_SOURCE_DIR}/smoke.sh) -add_ceph_test(unittest_bufferlist.sh ${CMAKE_SOURCE_DIR}/src/unittest_bufferlist.sh) # XXX are these safe to remove? they used to be around for ceph-disk tox testing set(env_vars_for_tox_tests diff --git a/src/test/bufferlist.cc b/src/test/bufferlist.cc index beb094fd831..7e5d3c45aff 100644 --- a/src/test/bufferlist.cc +++ b/src/test/bufferlist.cc @@ -44,67 +44,33 @@ static char cmd[128]; TEST(Buffer, constructors) { - bool ceph_buffer_track = get_env_bool("CEPH_BUFFER_TRACK"); unsigned len = 17; - uint64_t history_alloc_bytes = 0; - uint64_t history_alloc_num = 0; // // buffer::create // - if (ceph_buffer_track) { - EXPECT_EQ(0, buffer::get_total_alloc()); - } - { bufferptr ptr(buffer::create(len)); - history_alloc_bytes += len; - history_alloc_num++; EXPECT_EQ(len, ptr.length()); - if (ceph_buffer_track) { - EXPECT_EQ(len, (unsigned)buffer::get_total_alloc()); - EXPECT_EQ(history_alloc_bytes, buffer::get_history_alloc_bytes()); - EXPECT_EQ(history_alloc_num, buffer::get_history_alloc_num()); - } } // // buffer::claim_char // - if (ceph_buffer_track) { - EXPECT_EQ(0, buffer::get_total_alloc()); - } - { char* str = new char[len]; ::memset(str, 'X', len); bufferptr ptr(buffer::claim_char(len, str)); - if (ceph_buffer_track) { - EXPECT_EQ(len, (unsigned)buffer::get_total_alloc()); - EXPECT_EQ(history_alloc_bytes, buffer::get_history_alloc_bytes()); - EXPECT_EQ(history_alloc_num, buffer::get_history_alloc_num()); - } EXPECT_EQ(len, ptr.length()); EXPECT_EQ(str, ptr.c_str()); bufferptr clone = ptr.clone(); - history_alloc_bytes += len; - history_alloc_num++; EXPECT_EQ(0, ::memcmp(clone.c_str(), ptr.c_str(), len)); delete [] str; } // // buffer::create_static // - if (ceph_buffer_track) { - EXPECT_EQ(0, buffer::get_total_alloc()); - } - { char* str = new char[len]; bufferptr ptr(buffer::create_static(len, str)); - if (ceph_buffer_track) { - EXPECT_EQ(0, buffer::get_total_alloc()); - EXPECT_EQ(history_alloc_bytes, buffer::get_history_alloc_bytes()); - EXPECT_EQ(history_alloc_num, buffer::get_history_alloc_num()); - } EXPECT_EQ(len, ptr.length()); EXPECT_EQ(str, ptr.c_str()); delete [] str; @@ -112,19 +78,8 @@ TEST(Buffer, constructors) { // // buffer::create_malloc // - if (ceph_buffer_track) { - EXPECT_EQ(0, buffer::get_total_alloc()); - } - { bufferptr ptr(buffer::create_malloc(len)); - history_alloc_bytes += len; - history_alloc_num++; - if (ceph_buffer_track) { - EXPECT_EQ(len, (unsigned)buffer::get_total_alloc()); - EXPECT_EQ(history_alloc_bytes, buffer::get_history_alloc_bytes()); - EXPECT_EQ(history_alloc_num, buffer::get_history_alloc_num()); - } EXPECT_EQ(len, ptr.length()); // this doesn't throw on my x86_64 wheezy box --sage //EXPECT_THROW(buffer::create_malloc((unsigned)ULLONG_MAX), buffer::bad_alloc); @@ -132,79 +87,37 @@ TEST(Buffer, constructors) { // // buffer::claim_malloc // - if (ceph_buffer_track) { - EXPECT_EQ(0, buffer::get_total_alloc()); - } - { char* str = (char*)malloc(len); ::memset(str, 'X', len); bufferptr ptr(buffer::claim_malloc(len, str)); - if (ceph_buffer_track) { - EXPECT_EQ(len, (unsigned)buffer::get_total_alloc()); - EXPECT_EQ(history_alloc_bytes, buffer::get_history_alloc_bytes()); - EXPECT_EQ(history_alloc_num, buffer::get_history_alloc_num()); - } EXPECT_EQ(len, ptr.length()); EXPECT_EQ(str, ptr.c_str()); bufferptr clone = ptr.clone(); - history_alloc_bytes += len; - history_alloc_num++; EXPECT_EQ(0, ::memcmp(clone.c_str(), ptr.c_str(), len)); } // // buffer::copy // - if (ceph_buffer_track) { - EXPECT_EQ(0, buffer::get_total_alloc()); - } - { const std::string expected(len, 'X'); bufferptr ptr(buffer::copy(expected.c_str(), expected.size())); - history_alloc_bytes += len; - history_alloc_num++; - if (ceph_buffer_track) { - EXPECT_EQ(len, (unsigned)buffer::get_total_alloc()); - EXPECT_EQ(history_alloc_bytes, buffer::get_history_alloc_bytes()); - EXPECT_EQ(history_alloc_num, buffer::get_history_alloc_num()); - } EXPECT_NE(expected.c_str(), ptr.c_str()); EXPECT_EQ(0, ::memcmp(expected.c_str(), ptr.c_str(), len)); } // // buffer::create_page_aligned // - if (ceph_buffer_track) { - EXPECT_EQ(0, buffer::get_total_alloc()); - } - { bufferptr ptr(buffer::create_page_aligned(len)); - history_alloc_bytes += len; - history_alloc_num++; ::memset(ptr.c_str(), 'X', len); - if (ceph_buffer_track) { - EXPECT_EQ(len, (unsigned)buffer::get_total_alloc()); - EXPECT_EQ(history_alloc_bytes, buffer::get_history_alloc_bytes()); - EXPECT_EQ(history_alloc_num, buffer::get_history_alloc_num()); - } // doesn't throw on my x86_64 wheezy box --sage //EXPECT_THROW(buffer::create_page_aligned((unsigned)ULLONG_MAX), buffer::bad_alloc); #ifndef DARWIN ASSERT_TRUE(ptr.is_page_aligned()); #endif // DARWIN bufferptr clone = ptr.clone(); - history_alloc_bytes += len; - history_alloc_num++; EXPECT_EQ(0, ::memcmp(clone.c_str(), ptr.c_str(), len)); - if (ceph_buffer_track) { - EXPECT_EQ(history_alloc_bytes, buffer::get_history_alloc_bytes()); - EXPECT_EQ(history_alloc_num, buffer::get_history_alloc_num()); - } - } - if (ceph_buffer_track) { - EXPECT_EQ(0, buffer::get_total_alloc()); } } @@ -2930,7 +2843,7 @@ TEST(BufferHash, all) { /* * Local Variables: * compile-command: "cd .. ; make unittest_bufferlist && - * ulimit -s unlimited ; CEPH_BUFFER_TRACK=true valgrind \ + * ulimit -s unlimited ; valgrind \ * --max-stackframe=20000000 --tool=memcheck \ * ./unittest_bufferlist # --gtest_filter=BufferList.constructors" * End: diff --git a/src/unittest_bufferlist.sh b/src/unittest_bufferlist.sh deleted file mode 100755 index 5c0b62fe100..00000000000 --- a/src/unittest_bufferlist.sh +++ /dev/null @@ -1,19 +0,0 @@ -#!/bin/sh -# -# Ceph - scalable distributed file system -# -# Copyright (C) 2013 Cloudwatt -# -# Author: Loic Dachary -# -# This program is free software; you can redistribute it and/or modify -# it under the terms of the GNU Library Public License as published by -# the Free Software Foundation; either version 2, or (at your option) -# any later version. -# -# This program is distributed in the hope that it will be useful, -# but WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -# GNU Library Public License for more details. -# -CEPH_BUFFER_TRACK=true unittest_bufferlist -- 2.39.5