Add missing final keyword

[DimaMishchenko]

• Added missing final keyword
• Fixed small typo in MutableCollectionTests class name

Checklist

• I've read the Contribution Guidelines
• My contributions are licensed under the Swift license.
• I've followed the coding style of the rest of the project.
• I've added tests covering all new code paths my change adds to the project (if appropriate).
• I've added benchmarks covering new functionality (if appropriate).
• I've verified that my change does not break any existing tests or introduce unexplained benchmark regressions.
• I've updated the documentation if necessary.

[lorentey]

👍

I don't see anything wrong with adding explicit final modifiers, but to be honest I don't see why we'd need to add them either. Can you explain why are you suggesting this change?

E.g., is it only to make things explicit (like the way this codebase tends to spell out internal even though it's the default), or is there a practical effect I should know about?

[lorentey]

@swift-ci test

[DimaMishchenko]

@lorentey yes, sure.

• explicit declaration, as you mentioned
• small performance optimization by reducing dynamic dispatch

[lorentey]

@DimaMishchenko do you have a way to quantify the performance optimization? E.g., does it show up in benchmarks? Have you seen a codegen improvement?

[kylemacomber]

I think it makes sense to make public types final if they aren't intended to be subclassed. It's my understanding that Swift will "infer" the final when possible, for example on any non-public classes that don't have subclasses (at least if you build with WMO on).

Edit: @lorentey points out to me these types aren't open so they can't be subclasses outside of the module anyway.

[DimaMishchenko]

@lorentey first of all, sorry for long feedback.

What @kylemacomber said should be correct (at least according to this and this).

Which should mean that changes are only useful form code-style perspective.

But I did some additional performance testing for Deque (because _DequeBuffer was changed to final) to make sure that all works as expected.
Did comparison of Deque with final _DequeBuffer vs non-final.

First, simply tested append operation with performance test (using measure).

func testAppend() {
  var deque = Deque<Int>()
    
  let options = XCTMeasureOptions()
  options.iterationCount = 100
    
  measure {
    (0...1000000).forEach { _ in deque.append(1) }
  }
}

Results:

• Without WMO.
  • final - 0.775
  • non-final - 0.778 sec
• With WMO (-O)
  • final - 0.024
  • non-final - 0.026 sec

Then, benchmark test append (max-size - 16M, cycles - 20).

Difference is quite small (and can be just accuracy error).
Now for me it seems that changes are only useful to keep same style and do things explicit.

So @lorentey if you think that this PR is unnecessary, please feel free to close it 😀

[jjatie]

On the side of adding the final annotation: swift/OptimizationTips recommends it. Whether or not it has a meaningful impact for performance here, according to the linked article it is seems to be good general advice. IMHO official packages should follow guidelines/recommendations from the language.

FWIW I recently had an unrelated disagreement at work, which was resolved through examples from swift-algorithms. One of the many things I appreciate from the Swift community is that people are largely onboard with one set of best practices.

[lorentey]

The recommendation in OptimizationTips is mostly out of date. It was added in 2015, predating Swift 3's introduction of the open qualifier.