[SERVER-78013] Clean-up the use of empty DocumentStorage Created: 12/Jun/23 Updated: 14/Jun/23 |
|
| Status: | Backlog |
| Project: | Core Server |
| Component/s: | Internal Code |
| Affects Version/s: | None |
| Fix Version/s: | None |
| Type: | Improvement | Priority: | Major - P3 |
| Reporter: | Romans Kasperovics | Assignee: | Backlog - Query Execution |
| Resolution: | Unresolved | Votes: | 0 |
| Labels: | neweng, quick-tech-debt | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Issue Links: |
|
||||||||
| Assigned Teams: |
Query Execution
|
||||||||
| Participants: | |||||||||
| Linked BF Score: | 135 | ||||||||
| Description |
|
The DocumentStorage class offers a public static const DocumentStorage& DocumentStorage::emptyDoc() method which returns a reference to a global private static const DocumentStorage::DocumentStorage kEmptyDoc. This public method is only used in private const DocumentStorage& Document::storage(), which returns a reference to the same global variable for all documents with empty storage pointer. IMHO, the whole thing should be avoided, unless there are some measurable benefits (e.g., performance). As an additional complication, the way the global variable is initialized exposes it to the theoretically possible "static initialization order fiasco". As an additional complication, class DocumentStorage contains mutable data members (and data members modified with const_cast), which increases the chance of having a data race. Moreover, Document::_storage is already an (intrusive) pointer, why to have and additional storage() method instead of pointing to the empty storage directly? |