Perf: replace dynamic_cast with static_cast in this_thread()

[masaori335]

During the investigation of HTTP/2 performance regression, I realized that this dynamic_cast has a significant impact (~6%).
We never initialize Thread directly, so we don't need to use dynamic_cast here.

[maskit]

Are we never going to introduce any other subclasses?

[masaori335]

Derive another class from Thread? Event, EventProcessor, and EThread are tightly combined, so it might means a Thread sub class which not on the EventProcessor. Wil we have something like that?

[maskit]

I personally don't have the plan, but it is possible as long as we have Thread as a abstract class. If someone introduced XThread, it can be accidentally used as EThread with your change.

I think the objective of the function is not only the cast but also to check if we are on EThread. If you call it on XThread you cannot get an object, and this prevents the misuse.

[maskit]

I'm not sure if this is faster than dynamic_cast, but I'd say ok if the change was like this.

TS_INLINE EThread *
this_ethread()
{
  Thread *t = this_thread();
  if (t->type() == ETHREAD) {
    return static_cast<EThread *>(this_thread());
  } else {
    return nullptr;
  }
}

[masaori335]

Do you mean typeid operator? It might little faster than dynamic_cast. However, typeid still needs RTTI because Thread is a polymorphic type, so it still has runtime overhead.

In another function or class, I agree with adding some guards or use dynamic_cast. However, I doubt it's worth to pay any additional cost for assuming an almost impossible future & preparing un-existing issues in the critical path.

1. Technically, we can derive XThread from Thread. What is a real use case of XThread?
   I doubt the design of Thread & EThread. ( I start thinking about Thread is too generic ).
2. Even if we derive XThread, as you pointed, calling this_ethread() from XThread is obviously misuse. We need a new function to get what thread we're on.
3. Even if we derive XThread and call this_ethread() by some reason, we can add the guard at the time.

[oknet]

The this_thread() and this_ethread() should be wrap functions for pthread_getspecific() and just apply static_cast for the returned void * object. It is not necessary to perform dynamtic_cast on it.

[maskit]

If simple static_cast is what we need then I'm fine with it, but the behavior doesn't match with the comment in I_Thread.h. The comment have to be updated too.

[maskit]

Also, I'm not saying we should keep abstract Thread class. If you think we won't need XThread then you could remove the abstract class. It would remove all this confusion and complexity.

#6288 will fix the behavior

[vmamidi]

I think that we can't give up on performance for a use case that may or may not exist. If we have alternative approaches to achieve the same without impacting performance, we should do that; otherwise, this PR is good. I also agree that we can remove the Thread class, but I do not think that is the goal of this PR.

[maskit]

otherwise, this PR is good

I don't think so. Mismatch of comment and implementation will cause confusion. I think the comment in I_Thread.h should have been updated regardless of #6288.

[zwoop]

Cherry-picked to v9.0.x branch.