WBWI Internal Move implementation from .h into .cpp

[adamretter]

Moves some of the structural refactoring from #8135 into this PR.
This just cleans up the code by moving implementation out of the .h file and into the .cc file.

Should be considered for merge before both #7214 and #8135

[mrambacher]

LGTM.

[facebook-github-bot]

@mrambacher has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

[facebook-github-bot]

@mrambacher merged this pull request in 2760c2a.

[ajkr]

I think it deserves a benchmark comparison. Also we may benefit from declaring BaseDeltaIterator a final class.

[adamretter]

@ajkr I was surprised to see your suggestion for a benchmark here. I did not expect that moving the implementation from the .h file to the .cc file would have an impact. Have seen such issues in the past?

[ajkr]

I don't remember. More generally, we have seen a big difference in performance with vs. without inlining function calls. My guess is that this change doesn't affect inline-ability except in rare cases (e.g., build with profile-guided optimization and without link-time optimization - BaseDeltaIterator clients may speculatively devirtualize callsites to regular function calls, which can only be inlined when definitions are in the header).

I think it's hard to prove so wouldn't suggest trying much beyond benchmarks with default build settings. I can't think of why it would change in that case.