Skip to content

Commit

Permalink
Small FlatMap optimization with default initialization
Browse files Browse the repository at this point in the history
  • Loading branch information
chenBright committed Apr 27, 2024
1 parent 68aff69 commit dcdf77f
Show file tree
Hide file tree
Showing 13 changed files with 145 additions and 139 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/ci-linux.yml
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ jobs:
- name: compile tests
run: |
cd test
make -j ${{env.proc_num}}
make clean && make -j ${{env.proc_num}} brpc_builtin_service_unittest
- name: run tests
run: |
cd test
Expand Down
4 changes: 0 additions & 4 deletions src/brpc/acceptor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -217,10 +217,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
3 changes: 0 additions & 3 deletions src/brpc/http_header.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -82,9 +82,6 @@ std::string& HttpHeader::GetOrAddHeader(const std::string& key) {
return _content_type;
}

if (!_headers.initialized()) {
_headers.init(29);
}
return _headers[key];
}

Expand Down
13 changes: 3 additions & 10 deletions src/brpc/kvmap.h
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,11 @@ class KVMap {
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,13 +60,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
21 changes: 0 additions & 21 deletions src/brpc/server.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1998,17 +1998,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 @@ -2124,16 +2113,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
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
26 changes: 19 additions & 7 deletions src/butil/containers/flat_map.h
Original file line number Diff line number Diff line change
Expand Up @@ -151,12 +151,12 @@ class FlatMap {
key_type key;
};

FlatMap(const hasher& hashfn = hasher(),
const key_equal& eql = key_equal(),
const allocator_type& alloc = allocator_type());
explicit FlatMap(const hasher& hashfn = hasher(),
const key_equal& eql = key_equal(),
const allocator_type& alloc = allocator_type());
~FlatMap();
FlatMap(const FlatMap& rhs);
void operator=(const FlatMap& rhs);
FlatMap& operator=(const FlatMap& rhs);
void swap(FlatMap & rhs);

// Must be called to initialize this map, otherwise insert/operator[]
Expand Down Expand Up @@ -240,6 +240,7 @@ class FlatMap {
const_iterator restore_iterator(const PositionHint&) const;

// True if init() was successfully called.
// Always returns true.
bool initialized() const { return _buckets != NULL; }

bool empty() const { return _size == 0; }
Expand Down Expand Up @@ -277,9 +278,20 @@ class FlatMap {
template <typename _Map, typename _Element> friend class FlatMapIterator;
template <typename _Map, typename _Element> friend class SparseFlatMapIterator;
// True if buckets need to be resized before holding `size' elements.
inline bool is_too_crowded(size_t size) const
{ return size * 100 >= _nbucket * _load_factor; }

bool is_too_crowded(size_t size) const {
return size * 100 >= _nbucket * _load_factor;
}

// True if using default buckets which is for small map optimization.
bool is_default_buckets() const {
return _buckets == (Bucket*)(&_default_buckets_spaces);
}

static const size_t default_nbucket = 29;
// Small map optimization.
typename std::aligned_storage<sizeof(Bucket) * (default_nbucket + 1),
alignof(Bucket)>::type _default_buckets_spaces;
uint64_t _default_thumbnail[(default_nbucket + 63) / 64 * 8];
size_t _size;
size_t _nbucket;
Bucket* _buckets;
Expand Down
Loading

0 comments on commit dcdf77f

Please sign in to comment.