[SERVER-62791] Improve performance of WiredTigerRecoveryUnit::setTimestamp Created: 20/Jan/22  Updated: 29/Oct/23  Resolved: 07/Feb/22

Status: Closed
Project: Core Server
Component/s: None
Affects Version/s: None
Fix Version/s: 5.3.0

Type: Improvement Priority: Major - P3
Reporter: Donald Anderson Assignee: Jordi Olivares Provencio
Resolution: Fixed Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Depends
is depended on by WT-8366 API to set transaction commit timesta... Closed
Problem/Incident
Related
Backwards Compatibility: Fully Compatible
Sprint: Execution Team 2022-02-07, Execution Team 2022-02-21
Participants:
Linked BF Score: 135

 Description   

The performance of setTimestamp can be improved by using a stack buffer and simple formatting (like snprintf).

Rationale: setTimestamp sometimes pops up in performance critical paths (see HELP-28951).  In WT-8365, we improved the performance of the WT API used, and observed that simple changes on the caller side would improve performance more. See WT-8365 for discussion and benchmarks.



 Comments   
Comment by Githook User [ 07/Feb/22 ]

Author:

{'name': 'Jordi Olivares Provencio', 'email': 'jordi.olivares-provencio@mongodb.com', 'username': 'jordiolivares'}

Message: SERVER-62791 Modify setTimestamp to use stack allocation
Branch: master
https://github.com/mongodb/mongo/commit/c0aa22815e8540d1efc57425696778ba90df8160

Comment by Githook User [ 04/Feb/22 ]

Author:

{'name': 'Jordi Olivares Provencio', 'email': 'jordi.olivares-provencio@mongodb.com', 'username': 'jordiolivares'}

Message: Revert "SERVER-62791 Modify setTimestamp to use stack allocation"

This reverts commit 3f0788882675f8e4893a9c5ebbcee4716ed436fa.
Branch: master
https://github.com/mongodb/mongo/commit/bdb266d2b2149d0ccf0a89d7856f8e46205fdb07

Comment by Githook User [ 04/Feb/22 ]

Author:

{'name': 'Jordi Olivares Provencio', 'email': 'jordi.olivares-provencio@mongodb.com', 'username': 'jordiolivares'}

Message: SERVER-62791 Modify setTimestamp to use stack allocation
Branch: master
https://github.com/mongodb/mongo/commit/3f0788882675f8e4893a9c5ebbcee4716ed436fa

Comment by Donald Anderson [ 20/Jan/22 ]

louis.williams, sure, as long as it doesn't imply memory allocation.  Whatever unsignedHex calls is doing a new, which implies tcmalloc memory allocation, so it's running slow.

Comment by Donald Anderson [ 20/Jan/22 ]

Here's what I was thinking (untested code):

diff --git a/src/mongo/db/storage/wiredtiger/wiredtiger_recovery_unit.cpp b/src/mongo/db/storage/wiredtiger/wiredtiger_recovery_unit.cpp
index 19b40e4403..465813a449 100644
--- a/src/mongo/db/storage/wiredtiger/wiredtiger_recovery_unit.cpp
+++ b/src/mongo/db/storage/wiredtiger/wiredtiger_recovery_unit.cpp
@@ -847,8 +847,9 @@ Status WiredTigerRecoveryUnit::setTimestamp(Timestamp timestamp) {
         return Status::OK();
     }
 
-    const std::string conf = "commit_timestamp=" + unsignedHex(timestamp.asULL());
-    auto rc = session->timestamp_transaction(session, conf.c_str());
+    char conf[100];
+    snprintf(conf, sizeof(conf), "commit_timestamp=%" PRIx64, timestamp.asULL());
+    auto rc = session->timestamp_transaction(session, conf);
     if (rc == 0) {
         _isTimestamped = true;
     } 

Comment by Louis Williams [ 20/Jan/22 ]

We made some changes in the past (SERVER-41344) to replace sprintf with fmt/format.h and saw performance improvements, so we should consider using that library instead.

Generated at Thu Feb 08 05:56:04 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.