Summary:
The first parameter of SyncPoint::Process is "const std::string&". The majority, maybe all, of the actual calls to this function use a "const char *". The conversion before entering the function requires a construction of a std::string object on the heap. This std::object is then typically not needed because first use of the string is a rocksdb::Slice which has a less costly conversion of char * to slice.
Example:
We have a load and iterate test. The test loads 10m keys and iterates most via 10 rocksdb::Iterator objects. We used TCMALLOC to gather information about allocation and space usage during iterators.
- Before this PR: test took 32 min 17 sec
- After this PR: test took 1 min 14 sec
The TCMALLOC top object list before this PR:
<pre>
Total: 5105999 objects
5003717 98.0% 98.0% 5009471 98.1% rocksdb::DBIter::MergeValuesNewToOld (inline)
20260 0.4% 98.4% 20260 0.4% std::__cxx11::basic_string::_M_mutate
15214 0.3% 98.7% 15214 0.3% rocksdb::UncompressBlockContentsForCompressionType (inline)
13408 0.3% 99.0% 13408 0.3% std::_Rb_tree::_M_emplace_hint_unique [clone .constprop.416] (inline)
12957 0.3% 99.2% 12957 0.3% std::_Rb_tree::_M_emplace_hint_unique [clone .constprop.405] (inline)
9327 0.2% 99.4% 9327 0.2% std::_Rb_tree::_M_copy (inline)
7691 0.2% 99.5% 9919 0.2% JVM_FindSignal
2859 0.1% 99.6% 2859 0.1% rocksdb::Cleanable::RegisterCleanup
2844 0.1% 99.7% 2844 0.1% std::map::operator[] (inline)
</pre>
The "MergeValuesNewToOld (inline)" objects are the #define wrappers to SyncPoint::Process. We discovered this in a 5.18 rocksdb release. There TCMALLOC was more specific that std::basic_string was being constructed. I believe that was before SyncPoint::Process was declared inline in subsequent releases.
The TCMALLOC top object list after this PR:
<pre>
Total: 104911 objects
45090 43.0% 43.0% 45090 43.0% rocksdb::Cleanable::RegisterCleanup
29995 28.6% 71.6% 29995 28.6% rocksdb::LRUCacheShard::Insert
15229 14.5% 86.1% 15229 14.5% rocksdb::UncompressBlockContentsForCompressionType (inline)
4373 4.2% 90.3% 4551 4.3% JVM_FindSignal
2881 2.7% 93.0% 2881 2.7% rocksdb::::ReadBlockFromFile (inline)
1162 1.1% 94.1% 1176 1.1% rocksdb::BlockFetcher::ReadBlockContents (inline)
1036 1.0% 95.1% 1036 1.0% std::__cxx11::basic_string::_M_mutate
869 0.8% 95.9% 869 0.8% std::vector::_M_realloc_insert (inline)
806 0.8% 96.7% 806 0.8% SnmpAgent::GetVariables (inline)
</pre>
Pull Request resolved: https://github.com/facebook/rocksdb/pull/9023
Reviewed By: pdillinger
Differential Revision: D31610907
Pulled By: mrambacher
fbshipit-source-id: 574ff51b639dd46ad253a8e664a575f06b7cc85d
Summary:
Now SyncPoint is used in crash test but can signiciantly slow down the run. Add a bloom filter before each process to speed itup
Pull Request resolved: https://github.com/facebook/rocksdb/pull/8337
Test Plan: Run all existing tests
Reviewed By: ajkr
Differential Revision: D28730282
fbshipit-source-id: a187377a9d47877a36c5649e4b1f67d5e3033238
Summary:
When dynamically linking two binaries together, different builds of RocksDB from two sources might cause errors. To provide a tool for user to solve the problem, the RocksDB namespace is changed to a flag which can be overridden in build time.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/6433
Test Plan: Build release, all and jtest. Try to build with ROCKSDB_NAMESPACE with another flag.
Differential Revision: D19977691
fbshipit-source-id: aa7f2d0972e1c31d75339ac48478f34f6cfcfb3e
Summary:
There are too many types of files under util/. Some test related files don't belong to there or just are just loosely related. Mo
ve them to a new directory test_util/, so that util/ is cleaner.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/5377
Differential Revision: D15551366
Pulled By: siying
fbshipit-source-id: 0f5c8653832354ef8caa31749c0143815d719e2c
Summary:
In `db_stress` profile the vast majority of CPU time is spent acquiring the `SyncPoint` mutex. I mistakenly assumed #3939 had fixed this mutex contention problem by disabling `SyncPoint` processing. But actually the lock was still being acquired just to check whether processing is enabled. We can avoid that overhead by using an atomic to track whether it's enabled.
Closes https://github.com/facebook/rocksdb/pull/3991
Differential Revision: D8393825
Pulled By: ajkr
fbshipit-source-id: 5bc4e3c722ee7304e7a9c2439998c456b05a6897
matthewvon