[squid-dev] Efficient FD annotations

[Alex Rousskov]

For the record: The ideas below are superseded by the concept of the
code context introduced in commit ccfbe8f, including the
fde::codeContext field. --Alex


On 2/22/19 12:08 PM, Alex Rousskov wrote:
> In https://github.com/squid-cache/squid/pull/270#discussion_r259316609
> 
> 
>> In src/comm.cc:
>>
>>> @@ -424,7 +424,7 @@ comm_init_opened(const Comm::ConnectionPointer &conn,
>>      debugs(5, 5, HERE << conn << " is a new socket");
>>  
>>      assert(!isOpen(conn->fd));
>> -    fd_open(conn->fd, FD_SOCKET, note);
>> +    fd_open(conn->fd, FD_SOCKET, SBuf(note));
> 
> 
>> Alex: I do not think we should introduce this performance regression
>> on a common code path. Better debugging/reporting is just not worth
>> it. I have ideas on how this regression can be avoided (and debugging
>> improved), but I do not want to waste time detailing and discussing
>> them if you do not want to spend a lot more time on this PR.
> 
> 
>> Amos: Since this PR is about fixing the compile error I don't want to
>> complicate it any further. But do want to hear these ideas. Can you
>> post them to squid-dev so we can go over it separately please.
> 
> 
> IMO, the best way to annotate file descriptors (and other objects) for
> debugging/reporting purposes is to store pointers to their current
> owners and/or objects currently responsible for FD processing. Very
> roughly speaking:
> 
> class fde
> {
>   ...
> 
>   /* current FD handling context */
> 
>   /// Comm::Connection this FD belongs to
>   WeakConnectionPointer connection;
> 
>   /// Client that created this FD
>   WeakClientPointer creator;
> 
>   /// Server that accepted this FD
>   WeakServerPointer acceptor;
> 
>   /// generic FD owner/handler
>   /// (to cover exceptional contexts missed above?)
>   WeakFdOwnerPointer owner;
> 
>   /// poor man's generic FD owner/handler/operation description
>   /// (to cover performance-ignorant contexts missed above)
>   SBuf note;
> };
> 
> 
> Copying a pointer is a cheap operation; its performance expense is worth
> providing that extra information to developers and sysadmins. The heavy
> price of interpreting/reporting owner details is only paid when that
> extra information is actually requested/used, allowing us to provide a
> lot more useful details (than a short summary which we can compute and
> stuff into an SBuf every time we manipulate an FD).
> 
> Designing the right set of context pointers requires making difficult
> decisions such as whether the fde class should point to acceptor/creator
> (as shown in the above oversimplified sketch) or the Comm::Connection
> class should do that instead. FWIW, the latter makes more sense to me
> now, but I have not given it enough thought.
> 
> Implementing weak pointers correctly (including efficiently) OR
> convincing ourselves that certain strong pointers are acceptable in this
> context is also a serious challenge.
> 
> 
> IMO, we should be moving towards this design while continuing to provide
> sketchy/stale/truncated/limited FD info without performance regressions.
> 
> Fortunately, the two approaches can co-exist nicely: Want more/better
> details? Invest in the right approach for the context you are interested
> in. Just want to continue to provide the bare minimum? Continue to use
> expensive fixed annotation buffers (without slowing things down further)
> and/or cheap constant SBuf annotations.
> 
> 
> HTH,
> 
> Alex.
>