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. > _______________________________________________ squid-dev mailing list [email protected] http://lists.squid-cache.org/listinfo/squid-dev
