Slight adjust to the thread affinity logic

[duke8253]

Set thread affinity to current thread if the current thread type is the same as the target thread.

[shinrich]

Looks good. By biasing continuations to stay on the same thread on when operating within a pool, continuations associated with a transaction will be more likely to stay on the thread associated with the netvcs.

[SolidWallOfCode]

This is fine, but I will reiterate what we discussed in person - I want to see good documentation on how this works, both for future maintenance and for people writing code that uses this.

[duke8253]

Yes, docs are coming.

[oknet]

The EThread::schedule_*() is designed to reschedule the Cont into current EThread.

This PR makes the sub-Cont/SM to run in the EThread where the main-Cont/SM stays.
It can turn parallel tasks into serial tasks.

IMO, this is not a valuable change.

-1

[SolidWallOfCode]

That's a feature, not a bug. Because of the nature of the operations, the HttpSM is going to wait on the result regardless of what thread it uses. E.g., for a HostDB request, the HttpSm waits on the result from the HostDB, therefore doing that on a different thread will provide no parallelism. On the contrary, because these structures are locked, using multiple threads for a single transaction will introduce lock contention which will decrease performance. Having all the activity on the same thread for a transaction, along with the locks being reference counted, means no lock contention between the continuations. Related, it is important to note that network I/O is tied to a specific thread and it's very beneficial for performance to keep all HttpSM activity on that thread.

Finally, I would dispute the claim that EThread::schedule_*() was designed to reschedule the Continuation in to the current EThread - take a look at the definition for EventProcessor::assign_thread. It in effect picks a random thread, which means in a system with many cores and threads, it is very unlikely to pick the current EThread.

[scw00]

So can it save the original thread in some where . And HostDB callback to the original thread when dns searching successfully the same as what CacheVC does.

This pr involve a new feature but get rid of another feature.

[oknet]

@SolidWallOfCode

below is EThread::schedule(), it is different from EventProcessor::schedule_*().

TS_INLINE Event *
EThread::schedule(Event *e, bool fast_signal)
{
  e->ethread = this;
  ink_assert(tt == REGULAR);
  if (e->continuation->mutex) {
    e->mutex = e->continuation->mutex;
  } else {
    e->mutex = e->continuation->mutex = e->ethread->mutex;
  }
  ink_assert(e->mutex.get());
  EventQueueExternal.enqueue(e, fast_signal);
  return e;
}

Maybe you confuse EThread::schedule_*() and EventProcessor::schedule_*() ?

NetHandler is a batch processor for UnixNetVCs, it calls back NET Event to HttpSM one by one.

If one HttpSM schedule a Cont to current EThread, the Cont is called back only after all the NetVCs processed by related HttpSM.

To schedule an Event to an EThread which is picks up by round robin policy, the Cont maybe called back earlier than current EThread.

I can't describe the performance gap between them.

My advice is to keep the original code and use eventProcessor.schedule and ethread->schedule correctly & appropriately.

[SolidWallOfCode]

@scw00 That is what was done in various locations in a cut and paste fashion. It is, in my view, much more robust to have a common mechanism all of these places can use.

@oknet You may be correct, let me look again and then talk to Fei. I would agree that EThread::schedule should always schedule on that thread.

[SolidWallOfCode]

Now I'm more confused. This PR only changes EventProcessor::schedule. How does that change EThread::schedule?

In terms of an HttpSM scheduling a Continuation on another thread, I don't see the advantage. While using the current thread requires waiting for the other NetVC handling to be done, that's true of all ET_NET threads. Given the overhead of switching threads, it seems like a net negative. There's also the question of why the Continuation is being scheduled. If it's for lock contention, that determines the wait time more than which ET_NET thread is used. If it's waiting on some external event, the same applies. If, in fact, the work could be done immediately, then it should and not scheduled at all.