Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

shorten NIOThreadPool's thread names #1466

Merged
merged 1 commit into from
Mar 31, 2020
Merged

Conversation

weissi
Copy link
Member

@weissi weissi commented Mar 31, 2020

Motivation:

Previously, NIOThreadPool would use super long thread names. Linux
doesn't allow more than 15 bytes (+ \0 char) so in the end they would
never get set at all.

Modifications:

  • Shorten NIOThreadPool's thread names
  • Only take the first 15 characters of thread names in general on Linux

Result:

We don't lose thread names on Linux.

Motivation:

Previously, NIOThreadPool would use super long thread names. Linux
doesn't allow more than 15 bytes (+ \0 char) so in the end they would
never get set at all.

Modifications:

- Shorten NIOThreadPool's thread names
- Only take the first 15 characters of thread names in general on Linux

Result:

We don't lose thread names on Linux.
@weissi weissi requested review from Lukasa and glbrntt March 31, 2020 18:10
@Lukasa Lukasa added the 🔨 semver/patch No public API change. label Mar 31, 2020
@Lukasa Lukasa added this to the 2.16.0 milestone Mar 31, 2020
@Lukasa Lukasa merged commit b2d937b into apple:master Mar 31, 2020
pull bot pushed a commit to scope-demo/swift-nio that referenced this pull request Mar 31, 2020
Motivation:

Previously, NIOThreadPool would use super long thread names. Linux
doesn't allow more than 15 bytes (+ \0 char) so in the end they would
never get set at all.

Modifications:

- Shorten NIOThreadPool's thread names
- Only take the first 15 characters of thread names in general on Linux

Result:

We don't lose thread names on Linux.
@@ -175,7 +175,8 @@ public final class NIOThreadPool {

for id in 0..<self.numberOfThreads {
group.enter()
NIOThread.spawnAndRun(name: "NIOThreadPool thread #\(id)", detachThread: false) { thread in
// We should keep thread names under 16 characters because Linux doesn't allow more.
NIOThread.spawnAndRun(name: "TP-#\(id)", detachThread: false) { thread in
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know I missed the boat on this one, but there may be some value in having NIO in the name: "NIOTP-#\(id)" perhaps?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@glbrntt this time I deliberately left NIO out because we got so many bug reports about crashes if there's NIO in it :D . People thing that all crashes on NIO* threads are NIO's fault :|

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, very sensible then! 😅

@weissi weissi deleted the jw-tp-names branch April 1, 2020 11:26
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
🔨 semver/patch No public API change.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants