Skip to content

Home

Jump to bottom
Wesley Shillingford edited this page Dec 20, 2018 · 37 revisions

This lists changes I would like to make:

  • Replace std::bind with lambda:

https://stackoverflow.com/questions/15598607/when-should-i-use-stdbind

std::bind will probably be deprecated in the future https://stackoverflow.com/questions/33835922/why-should-bind-be-deprecated

Scott Meyers - Effective Modern C++ Item 34.

I understand the boost asio/beast examples have used it in the past, but it is going to be a deprecated feature soon.

https://youtu.be/zt7ThwVfap0?t=1511

For instance, boost callable traits: https://www.boost.org/doc/libs/1_68_0/libs/callable_traits/doc/html/index.html

https://github.com/nanocurrency/raiblocks/pull/1273/files

  • Wiki "Build instructions" incorrect boost path
  • Use flat_map, sorted vector or custom block allocator, instead of raw std::map. #define BOOST_POOL_NO_MT#include <boost/pool/pool_alloc.hpp>#include int main(){ typedef boost::fast_pool_allocator<int, boost::default_user_allocator_new_delete, boost::details::pool::default_mutex, 64, 128> allocator; std::list<int, allocator> l; for (int i = 0; i < 1000; ++i) l.push_back(i); l.clear(); boost::singleton_pool<boost::fast_pool_allocator_tag, sizeof(int)>:: purge_memory(); }

From https://theboostcpplibraries.com/boost.pool

  • Profile hot path and improve efficiencies there
  • Add code coverage reports. Code coverage with gcov & lcov? https://medium.com/@naveen.maltesh/generating-code-coverage-report-using-gnu-gcov-lcov-ee54a4de3f11
  • Add clang-tidy to pipeline
  • Use more std algorithms
  • Range loops, also boost::range?
  • Line length to clang format? Lines too long. Conditions can be factored out in if statements.
  • Replace std::function with auto, template or another inplaced version. It can have a SBO like std::string for small callables, otherwise otherwise it allocates.

If using callables as a function argument, then declare it as a template parameter.

From the SG14 (gaming) working group have an implementation: https://github.com/WG21-SG14/SG14/blob/master/SG14/inplace_function.h

Or even make your own: https://www.youtube.com/watch?v=VY83afAJUIg

  • Reorder member variables to reduce unnecessary packing and less cache misses by joining frequently used data together. Be careful about shared memory as it locks the whole cache line. https://stackoverflow.com/questions/892767/optimizing-member-variable-order-in-c
  • Common.hpp another place that namespace std is being polluted
  • Investigate if it is possible to run a multi-node (i.e run on both live and beta network at once).
  • .gitmodules references beast
  • Check that Big5 are noexcept where possible (especially move constructors)
  • Remove operator[] being used with std::map/std::unordered_map. See Effective C++ STL Item. Use find() and an InsertOrUpdate() method.
  • Avoid allocations. Consider using jemalloc. ::operator new(size_t) is most likely a wrapper around std::malloc in most implementations, which is good for large blocks (array/vector), but is inefficient for much smaller ones.
  • Backup representative in case your one goes down?
  • Are shared_ptrs abused?
  • Replace polymorphic inheritance, with static inheritance with templates?
  • Any branches which can be removed? branches which can be removed (make testing harder). if (type == UDP => ).
  • Avoid syntax like this: for (auto i (node.wallets.items.begin ()), n (node.wallets.items.end ()); i != n; ++i)

It is the old iterator style for loop, use a ranged for loop (I've seen them elsewhere in the code, but it should be consistent) for (auto wallet :: node.wallets.items) { }

  • Why is a std::deque used in node.cpp line 3271? This data structure is generally only used when it is desirable to insert at the beginning (in addition to the end), otherwise a std::vector is more beneficial.
  • 6 - Raw for loops don't show intent like a standard algorithm does, and I prefer to use an algorithm where possible. In the same area of code, replace the whole loop with something like: std::transform(roots.begin(), roots.end(), std::back_inserter(result), [](const auto& root) { return root->election->status.winner; }

Something like: for (auto & entry : list_probable_rep_weights ()) { result = result + entry.rep_weight.number (); }

Can be replaced with: auto result = std::accumulate(list_probable_rep_weights.begin(), list_probable_rep_weights.end(), static_castrai::uint128_t(0), [](auto count, auto& entry) { return count + entry.rep_weight.number (); }

Or even better with ranges v3 library it becomes: auto result = ranges::accumulate(list_probable_rep_weights, static_castrai::uint128_t(0), [](auto count, auto& entry) { return count + entry.rep_weight.number (); }

(I'm sure there's many other useful algorithms that can be used...).

  • Can precompiled headers speed up build? It's mostly dependencies which take time I think. Need to measure.
  • Too many public variables. Encapsulation, well defined inheritances...
  • rpc.hpp mrai_ should be swapped to be consistent. same with payment & work
  • Order of includes should be grouped by most dependent to least to make sure header files contain all necessary include dependencies to be parsed.
  • common.hpp - make deserialize virtual? seems strange it is not as seems part of the interface. to_bytes could return unique_ptr?

Clone this wiki locally