Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Small FlatMap optimization with default initialization #2620

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 1 addition & 6 deletions src/brpc/acceptor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -165,9 +165,8 @@ void Acceptor::StopAccept(int /*closewait_ms*/) {

int Acceptor::Initialize() {
if (_socket_map.init(INITIAL_CONNECTION_CAP) != 0) {
LOG(FATAL) << "Fail to initialize FlatMap, size="
LOG(WARNING) << "Fail to initialize FlatMap, size="
<< INITIAL_CONNECTION_CAP;
return -1;
}
return 0;
}
Expand Down Expand Up @@ -217,10 +216,6 @@ void Acceptor::ListConnections(std::vector<SocketId>* conn_list,
conn_list->reserve(ConnectionCount() + 10);

std::unique_lock<butil::Mutex> mu(_map_mutex);
if (!_socket_map.initialized()) {
// Optional. Uninitialized FlatMap should be iteratable.
return;
}
// Copy all the SocketId (protected by mutex) into a temporary
// container to avoid dealing with sockets inside the mutex.
size_t ntotal = 0;
Expand Down
1 change: 0 additions & 1 deletion src/brpc/builtin/hotspots_service.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,6 @@ static DisplayType StringToDisplayType(const std::string& val) {
static std::once_flag flag;
std::call_once(flag, []() {
display_type_map = new butil::CaseIgnoredFlatMap<DisplayType>;
display_type_map->init(10);
(*display_type_map)["dot"] = DisplayType::kDot;
#if defined(OS_LINUX)
(*display_type_map)["flame"] = DisplayType::kFlameGraph;
Expand Down
2 changes: 0 additions & 2 deletions src/brpc/controller.h
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,6 @@ friend void policy::ProcessThriftRequest(InputMessageBase*);
UserFieldsMap* request_user_fields() {
if (!_request_user_fields) {
_request_user_fields = new UserFieldsMap;
_request_user_fields->init(29);
}
return _request_user_fields;
}
Expand All @@ -276,7 +275,6 @@ friend void policy::ProcessThriftRequest(InputMessageBase*);
UserFieldsMap* response_user_fields() {
if (!_response_user_fields) {
_response_user_fields = new UserFieldsMap;
_response_user_fields->init(29);
}
return _response_user_fields;
}
Expand Down
6 changes: 2 additions & 4 deletions src/brpc/details/hpack.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -241,12 +241,10 @@ int IndexTable::Init(const IndexTableOptions& options) {
_need_indexes = options.need_indexes;
if (_need_indexes) {
if (_name_index.init(num_headers * 2) != 0) {
LOG(ERROR) << "Fail to init _name_index";
return -1;
LOG(WARNING) << "Fail to init _name_index";
}
if (_header_index.init(num_headers * 2) != 0) {
LOG(ERROR) << "Fail to init _name_index";
return -1;
LOG(WARNING) << "Fail to init _name_index";
}
}
if (options.static_table_size > 0) {
Expand Down
4 changes: 1 addition & 3 deletions src/brpc/details/naming_service_thread.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -441,9 +441,7 @@ int GetNamingServiceThread(
return -1;
}
if (g_nsthread_map->init(64) != 0) {
mu.unlock();
LOG(ERROR) << "Fail to init g_nsthread_map";
return -1;
LOG(WARNING) << "Fail to init g_nsthread_map";
}
}
NamingServiceThread*& ptr = (*g_nsthread_map)[key];
Expand Down
3 changes: 1 addition & 2 deletions src/brpc/extension.h
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,7 @@ class Extension {

private:
friend class butil::GetLeakySingleton<Extension<T> >;
Extension();
~Extension();
Extension() = default;
butil::CaseIgnoredFlatMap<T*> _instance_map;
butil::Mutex _map_mutex;
};
Expand Down
9 changes: 0 additions & 9 deletions src/brpc/extension_inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,15 +29,6 @@ Extension<T>* Extension<T>::instance() {
return butil::get_leaky_singleton<Extension<T> >();
}

template <typename T>
Extension<T>::Extension() {
_instance_map.init(29);
}

template <typename T>
Extension<T>::~Extension() {
}

template <typename T>
int Extension<T>::Register(const std::string& name, T* instance) {
if (NULL == instance) {
Expand Down
1 change: 0 additions & 1 deletion src/brpc/http_header.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ HttpHeader::HttpHeader()
, _method(HTTP_METHOD_GET)
, _version(1, 1)
, _first_set_cookie(NULL) {
CHECK_EQ(0, _headers.init(29));
// NOTE: don't forget to clear the field in Clear() as well.
}

Expand Down
20 changes: 10 additions & 10 deletions src/brpc/kvmap.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,14 +39,20 @@ class KVMap {
// Get value of a key(case-sensitive)
// Return pointer to the value, NULL on not found.
const std::string* Get(const char* key) const { return _entries.seek(key); }
const std::string* Get(const std::string& key) const { return _entries.seek(key); }
const std::string* Get(const std::string& key) const {
return _entries.seek(key);
}

// Set value of a key
void Set(const std::string& key, const std::string& value) { GetOrAdd(key) = value; }
void Set(const std::string& key, const char* value) { GetOrAdd(key) = value; }
void Set(const std::string& key, const std::string& value) {
_entries[key] = value;
}
void Set(const std::string& key, const char* value) { _entries[key] = value; }
// Convert other types to string as well
template <typename T>
void Set(const std::string& key, const T& value) { GetOrAdd(key) = std::to_string(value); }
void Set(const std::string& key, const T& value) {
_entries[key] = std::to_string(value);
}

// Remove a key
void Remove(const char* key) { _entries.erase(key); }
Expand All @@ -60,12 +66,6 @@ class KVMap {
size_t Count() const { return _entries.size(); }

private:
std::string& GetOrAdd(const std::string& key) {
if (!_entries.initialized()) {
_entries.init(29);
}
return _entries[key];
}

Map _entries;
};
Expand Down
4 changes: 0 additions & 4 deletions src/brpc/partition_channel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -400,10 +400,6 @@ class DynamicPartitionChannel::Partitioner : public NamingServiceWatcher {
if (options) {
_options = *options;
}
if (_part_chan_map.init(32, 70) != 0) {
LOG(ERROR) << "Fail to init _part_chan_map";
return -1;
}
return 0;
}

Expand Down
6 changes: 2 additions & 4 deletions src/brpc/policy/http2_rpc_protocol.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -362,12 +362,10 @@ H2Context::~H2Context() {

int H2Context::Init() {
if (_pending_streams.init(64, 70) != 0) {
LOG(ERROR) << "Fail to init _pending_streams";
return -1;
LOG(WARNING) << "Fail to init _pending_streams";
}
if (_hpacker.Init(_unack_local_settings.header_table_size) != 0) {
LOG(ERROR) << "Fail to init _hpacker";
return -1;
LOG(WARNING) << "Fail to init _hpacker";
}
return 0;
}
Expand Down
4 changes: 3 additions & 1 deletion src/brpc/policy/locality_aware_load_balancer.h
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,9 @@ class LocalityAwareLoadBalancer : public LoadBalancer {
butil::FlatMap<SocketId, size_t> server_map;

Servers() {
CHECK_EQ(0, server_map.init(1024, 70));
if (server_map.init(1024, 70) != 0) {
LOG(WARNING) << "Fail to init server_map";
}
}

// Add diff to left_weight of all parent nodes of node `index'.
Expand Down
12 changes: 9 additions & 3 deletions src/brpc/policy/rtmp_protocol.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -731,8 +731,12 @@ RtmpContext::RtmpContext(const RtmpClientOptions* copt, const Server* server)
_service = server->options().rtmp_service;
}
_free_ms_ids.reserve(32);
CHECK_EQ(0, _mstream_map.init(1024, 70));
CHECK_EQ(0, _trans_map.init(1024, 70));
if (_mstream_map.init(1024, 70) != 0) {
LOG(FATAL) << "Fail to initialize _mstream_map";
}
if (_trans_map.init(1024, 70) != 0) {
LOG(FATAL) << "Fail to initialize _trans_map";
}
memset(static_cast<void*>(_cstream_ctx), 0, sizeof(_cstream_ctx));
}

Expand Down Expand Up @@ -1766,7 +1770,9 @@ static pthread_once_t s_cmd_handlers_init_once = PTHREAD_ONCE_INIT;
static void InitCommandHandlers() {
// Dispatch commands based on "Command Name".
s_cmd_handlers = new CommandHandlerMap;
CHECK_EQ(0, s_cmd_handlers->init(64, 70));
if (s_cmd_handlers->init(64, 70) != 0) {
LOG(WARNING) << "Fail to init s_cmd_handlers";
}
(*s_cmd_handlers)[RTMP_AMF0_COMMAND_CONNECT] = &RtmpChunkStream::OnConnect;
(*s_cmd_handlers)[RTMP_AMF0_COMMAND_ON_BW_DONE] = &RtmpChunkStream::OnBWDone;
(*s_cmd_handlers)[RTMP_AMF0_COMMAND_RESULT] = &RtmpChunkStream::OnResult;
Expand Down
43 changes: 2 additions & 41 deletions src/brpc/server.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -118,8 +118,6 @@ DEFINE_bool(enable_threads_service, false, "Enable /threads");
DECLARE_int32(usercode_backup_threads);
DECLARE_bool(usercode_in_pthread);

const int INITIAL_SERVICE_CAP = 64;
const int INITIAL_CERT_MAP = 64;
// NOTE: never make s_ncore extern const whose ctor seq against other
// compilation units is undefined.
const int s_ncore = sysconf(_SC_NPROCESSORS_ONLN);
Expand Down Expand Up @@ -661,22 +659,6 @@ int Server::InitializeOnce() {
if (_status != UNINITIALIZED) {
return 0;
}
if (_fullname_service_map.init(INITIAL_SERVICE_CAP) != 0) {
LOG(ERROR) << "Fail to init _fullname_service_map";
return -1;
}
if (_service_map.init(INITIAL_SERVICE_CAP) != 0) {
LOG(ERROR) << "Fail to init _service_map";
return -1;
}
if (_method_map.init(INITIAL_SERVICE_CAP * 2) != 0) {
LOG(ERROR) << "Fail to init _method_map";
return -1;
}
if (_ssl_ctx_map.init(INITIAL_CERT_MAP) != 0) {
LOG(ERROR) << "Fail to init _ssl_ctx_map";
return -1;
}
_status = READY;
return 0;
}
Expand Down Expand Up @@ -2028,17 +2010,6 @@ int Server::AddCertificate(const CertInfo& cert) {
}

bool Server::AddCertMapping(CertMaps& bg, const SSLContext& ssl_ctx) {
if (!bg.cert_map.initialized()
&& bg.cert_map.init(INITIAL_CERT_MAP) != 0) {
LOG(ERROR) << "Fail to init _cert_map";
return false;
}
if (!bg.wildcard_cert_map.initialized()
&& bg.wildcard_cert_map.init(INITIAL_CERT_MAP) != 0) {
LOG(ERROR) << "Fail to init _wildcard_cert_map";
return false;
}

for (size_t i = 0; i < ssl_ctx.filters.size(); ++i) {
const char* hostname = ssl_ctx.filters[i].c_str();
CertMap* cmap = NULL;
Expand Down Expand Up @@ -2109,8 +2080,8 @@ int Server::ResetCertificates(const std::vector<CertInfo>& certs) {
}

SSLContextMap tmp_map;
if (tmp_map.init(INITIAL_CERT_MAP) != 0) {
LOG(ERROR) << "Fail to initialize tmp_map";
if (tmp_map.init(certs.size() + 1) != 0) {
LOG(ERROR) << "Fail to init tmp_map";
return -1;
}

Expand Down Expand Up @@ -2154,16 +2125,6 @@ int Server::ResetCertificates(const std::vector<CertInfo>& certs) {
}

bool Server::ResetCertMappings(CertMaps& bg, const SSLContextMap& ctx_map) {
if (!bg.cert_map.initialized()
&& bg.cert_map.init(INITIAL_CERT_MAP) != 0) {
LOG(ERROR) << "Fail to init _cert_map";
return false;
}
if (!bg.wildcard_cert_map.initialized()
&& bg.wildcard_cert_map.init(INITIAL_CERT_MAP) != 0) {
LOG(ERROR) << "Fail to init _wildcard_cert_map";
return false;
}
bg.cert_map.clear();
bg.wildcard_cert_map.clear();

Expand Down
4 changes: 3 additions & 1 deletion src/brpc/server_id.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,9 @@ namespace brpc {

ServerId2SocketIdMapper::ServerId2SocketIdMapper() {
_tmp.reserve(128);
CHECK_EQ(0, _nref_map.init(128));
if (_nref_map.init(128) != 0) {
LOG(WARNING) << "Fail to init _nref_map";
}
}

ServerId2SocketIdMapper::~ServerId2SocketIdMapper() {
Expand Down
6 changes: 0 additions & 6 deletions src/brpc/uri.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -72,9 +72,6 @@ static void ParseQueries(URI::QueryMap& query_map, const std::string &query) {
}
for (QuerySplitter sp(query.c_str()); sp; ++sp) {
if (!sp.key().empty()) {
if (!query_map.initialized()) {
query_map.init(URI::QUERY_MAP_INITIAL_BUCKET);
}
std::string key(sp.key().data(), sp.key().size());
std::string value(sp.value().data(), sp.value().size());
query_map[key] = value;
Expand Down Expand Up @@ -347,9 +344,6 @@ void URI::PrintWithoutHost(std::ostream& os) const {
}

void URI::InitializeQueryMap() const {
if (!_query_map.initialized()) {
CHECK_EQ(0, _query_map.init(QUERY_MAP_INITIAL_BUCKET));
}
ParseQueries(_query_map, _query);
_query_was_modified = false;
_initialized_query_map = true;
Expand Down
1 change: 0 additions & 1 deletion src/brpc/uri.h
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ namespace brpc {
// interpretable as extension
class URI {
public:
static const size_t QUERY_MAP_INITIAL_BUCKET = 16;
typedef butil::FlatMap<std::string, std::string> QueryMap;
typedef QueryMap::const_iterator QueryIterator;

Expand Down
4 changes: 3 additions & 1 deletion src/bthread/mutex.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,9 @@ void ContentionProfiler::init_if_needed() {
if (!_init) {
// Already output nanoseconds, always set cycles/second to 1000000000.
_disk_buf.append("--- contention\ncycles/second=1000000000\n");
CHECK_EQ(0, _dedup_map.init(1024, 60));
if (_dedup_map.init(1024, 60) != 0) {
LOG(WARNING) << "Fail to initialize dedup_map";
}
_init = true;
}
}
Expand Down
Loading
Loading