cygrunsrv + sshd + rsync = 20 times too slow -- throttled?

Corinna Vinschen corinna-cygwin@cygwin.com
Wed Sep 8 18:28:42 GMT 2021


On Sep  8 13:43, Ken Brown wrote:
> On 9/8/2021 7:55 AM, Corinna Vinschen wrote:
> > On Sep  8 20:32, Takashi Yano wrote:
> > > As for this patch, read_mtx was introduced. This handle is initialized
> > > only for read pipe. However, this seems to be NULL even without
> > > initialization in write pipe. I wonder why initializing read_mtx in
> > > the constructor is not necessary.
> > > 
> > > How do you guarantee that read_mtx is NULL on the write pipe?
> > 
> > fhandlers are always calloc'ed on the cygheap, so you don't have
> > to initialize anything to NULL.  It doesn't hurt, of course, but
> > it's certainly not required.
> 
> Takashi and I both asked this question, and you had to answer it twice.
> It's likely that future readers of the code will ask it again.

Yes, but ... it's not hidden knowledge.  You're using the function
build_fh_dev.  Looking into dtable.cc shows build_fh_dev calls
build_fh_pc which in turn calls fh_alloc.  fh_alloc uses the cnew macro,
defined in the same file, which resolves into

  void* ptr = (void*) ccalloc (HEAP_FHANDLER, 1, sizeof (fhandler_pipe));
  new (ptr) fhandler_pipe (...);

So the constructor is called on a memory slot on the cygheap, allocated
with ccalloc, the cygheap equivalent to calloc on the user heap.

> Would you be
> amenable to a code cleanup patch that answers it once and for all?  I would
> suggest (a) removing all 0/NULL initializers from fhandler constructors and
> (b) adding comments explaining why they're not needed.

a) ok

b) Commenting on the same issue in every single fhandler_* file appears a
   bit excessive, IMHO.  Only one comment in fhandler.h should suffice.


Corinna


More information about the Cygwin-developers mailing list