Make *-inl.h slim again

[bnoordhuis]

Inspired by me setting a breakpoint on a function and gdb telling me there are 12 locations...

The idea behind the *-inl.h files is to provide inline definitions of short functions. Large gobs of code don't belong in them because they negatively impact:

1. Compile times
2. Binary size (duplication)
3. Run-time performance (instruction cache pressure)

Rules of thumb:

1. Functions with few or infrequent callers should (judgment call) live in a .cc file
2. Functions over 4-5 lines that aren't template functions should live in a .cc file
3. Macros like CHECK(..) expand to multiple lines of code and should be counted as such

Good (short, many callers): https://github.com/nodejs/node/blob/7d13f5e34f7a9e98b925d7fdf77ebfadb7ed17d8/src/env-inl.h#L56-L58

Not good (big, only two infrequent callers): https://github.com/nodejs/node/blob/7d13f5e34f7a9e98b925d7fdf77ebfadb7ed17d8/src/env-inl.h#L349-L365

Questionable (single infrequent caller): https://github.com/nodejs/node/blob/7d13f5e34f7a9e98b925d7fdf77ebfadb7ed17d8/src/env-inl.h#L510-L518

The most commonly used *-inl.h files are:

$ find src -type f | xargs perl -ne 'print "$1\n" if /#\s*include "([^"]+-inl.h)"/' | sort | uniq -c | sort -r | head -9
  91 env-inl.h
  74 util-inl.h
  52 memory_tracker-inl.h
  42 base_object-inl.h
  35 async_wrap-inl.h
  29 debug_utils-inl.h
  19 threadpoolwork-inl.h
  18 node_process-inl.h
  15 stream_base-inl.h

[kvakil]

I ran rtags and hacked together something to put the results into SQLite. The line count is approximate (it's literally based on the number of visual lines the function takes up), but looks to already identify some big ones:

non-template inlined functions which are >= 8 lines

```console > select function_name, file || ':' || line || ':' || column as location, callers, number_of_lines from inlines where function_name NOT LIKE '%<%' and number_of_lines >= 8; function_name location callers number_of_lines -------------------------------------------------- ------------------------------ ---------- --------------- unsigned long base64_encode src/base64-inl.h:124:15 5 65 node::StreamWriteResult StreamBase::Write src/stream_base-inl.h:163:31 7 55 void NodeTraceStateObserver::OnTraceEnabled src/node_v8_platform-inl.h:25: 2 42 int StreamBase::Shutdown src/stream_base-inl.h:125:17 6 36 void StreamResource::RemoveStreamListener src/stream_base-inl.h:70:22 8 22 void SwapBytes16 src/util-inl.h:216:6 6 21 void SwapBytes32 src/util-inl.h:239:6 1 21 void SwapBytes64 src/util-inl.h:262:6 1 21 node::MemoryRetainerNode *MemoryTracker::AddNode src/memory_tracker-inl.h:302:3 1 18 node::fs::FSReqBase *GetReqWrap src/node_file-inl.h:233:12 37 18 void V8Platform::Initialize src/node_v8_platform-inl.h:87: 1 18 node::Environment *Environment::GetCurrent src/env-inl.h:179:34 93 16 void MemoryTracker::Track src/memory_tracker-inl.h:273:2 4 15 void V8Platform::StartTracingAgent src/node_v8_platform-inl.h:126 2 15 void ThreadPoolWork::ScheduleWork src/threadpoolwork-inl.h:32:22 4 15 unsigned long Histogram::RecordDelta src/histogram-inl.h:82:21 2 14 void FSReqBase::Init src/node_file-inl.h:51:17 1 14 void StreamReq::Done src/stream_base-inl.h:281:17 11 14 void V8Platform::Dispose src/node_v8_platform-inl.h:107 2 13 MemoryRetainerNode::MemoryRetainerNode src/memory_tracker-inl.h:24:10 1 10 void MemoryTracker::TrackField src/memory_tracker-inl.h:98:21 23 10 node::MemoryRetainerNode *MemoryTracker::AddNode src/memory_tracker-inl.h:322:3 3 9 SlicedArguments::SlicedArguments src/util-inl.h:504:18 2 9 bool Histogram::Record src/histogram-inl.h:72:17 1 8 const char *GetNodeName src/memory_tracker-inl.h:12:20 3 8 MemoryRetainerNode::MemoryRetainerNode src/memory_tracker-inl.h:36:10 1 8 void StreamResource::PushStreamListener src/stream_base-inl.h:60:22 8 8 bool StringEqualNoCaseN src/util-inl.h:315:6 13 8 bool IsSafeJsInt src/util-inl.h:549:13 11 8 bool FastStringKey::operator== src/util-inl.h:573:31 1 8 ```

Also can look at number of callers. There are some mistakes here, f.ex. some are marked as having zero callers when they clearly have some.

non-template inlined functions with <= 2 callsites

```console > select function_name, file || ':' || line || ':' || column as location, callers, number_of_lines from inlines where function_name NOT LIKE '%<%' and callers <= 2 order by number_of_lines desc limit 30; function_name location callers number_of_lines -------------------------------------------------- ------------------------------ ---------- --------------- void NodeTraceStateObserver::OnTraceEnabled src/node_v8_platform-inl.h:25: 2 42 void SwapBytes32 src/util-inl.h:239:6 1 21 void SwapBytes64 src/util-inl.h:262:6 1 21 node::MemoryRetainerNode *MemoryTracker::AddNode src/memory_tracker-inl.h:302:3 1 18 void V8Platform::Initialize src/node_v8_platform-inl.h:87: 1 18 void V8Platform::StartTracingAgent src/node_v8_platform-inl.h:126 2 15 unsigned long Histogram::RecordDelta src/histogram-inl.h:82:21 2 14 void FSReqBase::Init src/node_file-inl.h:51:17 1 14 void V8Platform::Dispose src/node_v8_platform-inl.h:107 2 13 MemoryRetainerNode::MemoryRetainerNode src/memory_tracker-inl.h:24:10 1 10 SlicedArguments::SlicedArguments src/util-inl.h:504:18 2 9 bool Histogram::Record src/histogram-inl.h:72:17 1 8 MemoryRetainerNode::MemoryRetainerNode src/memory_tracker-inl.h:36:10 1 8 bool FastStringKey::operator== src/util-inl.h:573:31 1 8 bool Environment::no_browser_globals src/env-inl.h:675:26 2 7 double Histogram::Add src/histogram-inl.h:20:19 1 7 FSReqBase::FSReqBase src/node_file-inl.h:42:12 1 7 unsigned long FastStringKey::HashImpl src/util-inl.h:559:33 1 7 bool BaseObject::IsWeakOrDetached src/base_object-inl.h:87:18 2 6 void Environment::DoneBootstrapping src/env-inl.h:621:26 2 6 void MemoryTracker::TrackInlineFieldWithSize src/memory_tracker-inl.h:84:21 1 6 node::MemoryRetainerNode *MemoryTracker::PushNode src/memory_tracker-inl.h:340:3 2 6 FSReqCallback::FSReqCallback src/node_file-inl.h:77:16 1 6 void StreamReq::AttachToObject src/stream_base-inl.h:20:17 1 6 void BaseObject::ClearWeak src/base_object-inl.h:80:18 2 5 DiagnosticFilename::DiagnosticFilename src/diagnosticfilename-inl.h:1 1 5 void ShouldNotAbortOnUncaughtScope::Close src/env-inl.h:406:37 2 5 node::BaseObject *CleanupHookCallback::GetBaseObje src/env-inl.h:814:34 1 5 void MemoryTracker::TrackInlineField src/memory_tracker-inl.h:290:2 0 5 ```

Hopefully this provides a burndown list for people looking to work on this.

Here is a (gzipped) copy of the database for those looking for a finer-grained analysis: inl.db.gz

[lilsweetcaligula]

@bnoordhuis I have refactored stream_base-inl.h and managed to get the binary size down by 4 bytes (in 9 compilations out of 10). Would you accept a PR?

[bnoordhuis]

@lilsweetcaligula I'm going to guess most calls come from src/stream_base.cc if moving code around only slims down the binary by 4 bytes. But sure, pull request welcome.