[SERVER-31164] Stop swapping thread name and client in thread-per-connection mode Created: 19/Sep/17 Updated: 27/Oct/23 Resolved: 06/Nov/17 |
|
| Status: | Closed |
| Project: | Core Server |
| Component/s: | Networking |
| Affects Version/s: | None |
| Fix Version/s: | None |
| Type: | Improvement | Priority: | Major - P3 |
| Reporter: | Mathias Stearn | Assignee: | DO NOT USE - Backlog - Platform Team |
| Resolution: | Gone away | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Backwards Compatibility: | Fully Compatible |
| Participants: |
| Description |
|
This appears to cost about 3% in the Insert.JustId benchmark. |
| Comments |
| Comment by Jonathan Reams [ 06/Nov/17 ] | ||||||||||||||||
|
This was done in | ||||||||||||||||
| Comment by Jonathan Reams [ 18/Oct/17 ] | ||||||||||||||||
|
After talking in person, I'm going to send this to the backlog and make it 3.7 desired. | ||||||||||||||||
| Comment by Jonathan Reams [ 17/Oct/17 ] | ||||||||||||||||
|
Why? We can change the ThreadGuard to say
and then the same in the destructor:
That should be two short string comparisons, and we cache the thread name so getThreadName() doesn't need to make any syscalls. | ||||||||||||||||
| Comment by Mathias Stearn [ 17/Oct/17 ] | ||||||||||||||||
|
I don't think that will work in this case because the ThreadGuard will always be changing the thread name back and forth. Maybe instead just have a server parameter for profiling that disables the setThreadName calls in ThreadGuard. | ||||||||||||||||
| Comment by Jonathan Reams [ 17/Oct/17 ] | ||||||||||||||||
|
That all makes sense to me. I'd be for conditionally setting the thread name. Maybe only setThreadName() if oldThreadName() != newThreadName? | ||||||||||||||||
| Comment by Mathias Stearn [ 17/Oct/17 ] | ||||||||||||||||
|
The timing was based on the percent of time linux perf attributed to the ThreadGuard functions. Most of the time was in setting the thread name. I just took a look at the linux sources and that could just be an artifact introduced by setting the thread name directly interacting with perf: https://github.com/torvalds/linux/blob/v4.13/fs/exec.c#L1259. I think it is possible that that causes all cycles that are near the thread-name change to be attributed to changing the thread name to ensure that they go to the correct thread name. I just tried directly measuring the impact by commenting out the calls to setThreadName() out and the impact was ~1% which is close enough to the noise floor that I'm not sure if it is real. Given that, I think the only justification for doing this would be to avoid this interaction with the profiler, and I'm not even sure that is worth it. | ||||||||||||||||
| Comment by Jonathan Reams [ 17/Oct/17 ] | ||||||||||||||||
|
I'm curious what's actually expensive here. This should be copying less than 8 bytes into a thread-local? Maybe an allocation? I agree it doesn't make sense to swap the thread name if it's always going to be the same, but I don't understand why it counts for a 3% perf hit. |