[squid-dev] Efficient FD annotations

Alex Rousskov rousskov at measurement-factory.com
Fri Feb 22 17:08:16 UTC 2019


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.



More information about the squid-dev mailing list