This relates to the BSONElement checks added in SERVER-xxx, where I found that the regressions to the update command could be reversed.

tl;dr don't construct unnecessary BSONElements in update command path.

FieldAtOffset Regression investigation

I've taken a look at the workload a bit more with the help of AI. The flame graphs (post changes) show a different shape starting with `mutablebson::Element::findFirstChildNamed`, which takes up a large percentage of the cycles profiled. We see large chunks of our new `BSONElement` method as separate frames in different areas, which leads me to think we may be constructing `BSONElement` multiple times for the same element.

Code walkthrough

Element Element::findFirstChildNamed(std::string_view name) const {
    ...
    while ((current != kInvalidRepIdx) && (impl.getFieldName(impl.getElementRep(current)) != name))
        current = impl.resolveRightSibling(current);
}

There are 3 places in this function where we construct a new `BSONElement` for the same element data:
1. `findFirstChildNamed` calls-> `impl.getFieldName(...)` calls-> `getSerializedElement(rep)` calls-> `BSONElement::fieldNameStringData` on the element.
    * This `BSONElement` construction could be avoided. `ElementRep` already stores a `_fieldNameSize`.
2. `findFirstChildNamed` calls-> `impl.resolveRightSibling` calls-> `getSerializedElement(rep)` calls-> `BSONElement::size()`
    * This `BSONElement` construction could be avoided. `ElementRep` can store a `_totalSize` now. It actually used to do this: https://github.com/10gen/mongo/blob/8505d6092a6680179d27e1b8bf103a4e7577c3e6/src/mongo/bson/mutable/document.cpp#L496
3. `findFirstChildNamed` calls-> `impl.resolveRightSibling` calls-> `BSONElement(...)` to call `BSONElement::eoo`, `BSONElement::totalSize`, `BSONElement::fieldNameSize`
    * I'm not sure if we should remove the construction here. There is type inspection and we should probably "validate" the data at least once with our checks.

We don't actually need the full capability of `BSONElement` in most of these circumstances. We need something like `fieldNameSize` or `totalSize`, so a full `BSONElement` construction isn't necessary.

tl;dr we construct BSONElement 3 times during the findFirstChildNamed scan for every element

POC solution

I avoid BSONElement construction in 1 (`impl.getFieldName`) and 2 (`resolveRightSibling`). This should earn us back some perf (even on master where we don't have any of the extra `BSONElement` validation) because we remove `BSONElement` construction. We don't have to change these methods here if we're worried about impact to other call sites. We could use dedicated helper methods for this `findFirstChildNamed` scan.

[link to code changes](https://app.graphite.com/github/pr/10gen/mongo/61893)

Here are the perf numbers: comparison link

Example numbers (thread level 1):

tl;dr < 3.5% regression on all thread levels

Is this a valid change, or am I just taking shortcuts to reduce benchmark numbers?

I think avoiding the 3x `BSONElement` construction for each element during a scan should definitely be a required fix even separately from this PR.

I'm not sure if I did the right change for the `mutable_bson` API though. I essentially did a partial revert to what `mutable_bson` used to be: https://github.com/10gen/mongo/blob/8505d6092a6680179d27e1b8bf103a4e7577c3e6/src/mongo/bson/mutable/document.cpp#L496
which stored both `fieldNameSize` and `totalSize`. I have some history on why `totalSize` was removed, lmk if interested.

tl;dr more work or review would be required to fit the change into the API correctly