NFD » ndn-cxx

The "guidelines" have nothing to do with "size bloat". These guidelines are for the chromium project and I see no reason to adopt them.

There is no point of changing the current code, except wasting time and making excessive number of compilation units, wasting more time on the library compilation. The code "bloat" is irrelevant to the objectives of the project. The binary size will be large, since we are using many things from STL and boost that produce a lot of code (e.g., boost asio).

Insignificant change. No reason to waste time on doing any updates.

The compilation time wouldn't change for just one change. But the consequence of your push is total removal of all inline-only classes, which is extremely bad idea.

I'm not changing the fact the most of the library stay as header only. This includes name, name-component, most of management, and many other core classes.

Compilation time is less important than binary size.
There's also no proof than compilation time is increased significantly.

Larger binary => more runtime memory consumption => (for embedded NFD) less memory for ContentStore.

Alex Afanasyev wrote:

There is no point of changing the current code, except wasting time and making excessive number of compilation units, wasting more time on the library compilation.

Actually, the opposite is true. More code in header files means that the compiler must go through a lot of the same code several times (once for each translation unit that directly or indirectly includes that header file), therefore increasing total build time. Precompiled headers may alleviate this issue but still won't reduce it to zero.

Moreover, the compiler will generate tons of duplicate symbols, which then the linker will have to merge and discard, further increasing build time.

I'm not changing the fact the most of the library stay as header only.

This is not a valid point IMHO. Having a header-only library could be a goal (however, who decided that it's a goal? and why?). But a "mostly" header-only library doesn't make sense, either all or nothing.

But the consequence of your push is total removal of all inline-only classes, which is extremely bad idea.

Why? You keep saying this without stating a provable reason.

20140708 conference call decides we should stop adding new inline functions unless they are trivial getters/setters.

Fixing old code is low priority, and @Beichuan can decide when to do it.

@Lixia Zhang points out "who cares" is not a right attitude toward this problem, because code size bloat does cause problem in small devices such as home routers.

Please give the exact problem. What is the "target size"?

The few bytes that can be saved by non-inlining could be nothing compared to the amount of space you can save by switching to use shared library.

note-6 decision was made one year ago, and it has been caused numerous disputes.

I suggest amending the rule as:

• In general, don't use inline.
• Constructors and destructors SHOULD NOT be inline (except template).
• The following MAY be inline:
  • trivial getter, ie. return m_field; where m_field is a TriviallyCopyable type
  • trivial setter, ie. m_field = arg; where m_field is a TriviallyCopyable type
  • forwarding function, ie. call another function with same arguments; note: arguments should be TriviallyCopyable or accepted by const T&
  • small function body: no more than 1 function call (not counting calls to trivial getter/setter; including overloaded operator), no more than 7 operators, no control statements (such as if switch for while throw try)
• A virtual method can be inline if satisfying above conditions.

20150801 conference call did not approve note-8.

The only approved rule is still note-6, which Alex very reluctantly agreed to a year ago.

However, I will use the loosen rules in note-8 for future code reviews.