Re: [PATCH 1/1] hurd: Avoid extra ctty RPCs in init_dtable ()

[Samuel Thibault <samuel.thibault@gnu.org>]

Applied, thanks!

Sergey Bugaev, le sam. 15 avril 2023 19:17:18 +0300, a ecrit:
> It is common to have (some of) stdin, stdout and stderr point to the
> very same port. We were making the ctty RPCs that _hurd_port2fd () does
> for each one of them separately:
> 
> 1. term_getctty ()
> 2. mach_port_deallocate ()
> 3. term_open_ctty ()
> 
> Instead, let's detect this case and duplicate the ctty port we already
> have. This means we do 1 RPC instead of 3 (and create a single protid
> on the server side) if the file is our ctty, and no RPCs instead of 1
> if it's not. A clear win!
> 
> Signed-off-by: Sergey Bugaev <bugaevc@gmail.com>
> ---
>  hurd/dtable.c | 46 +++++++++++++++++++++++++++++++++++++++-------
>  1 file changed, 39 insertions(+), 7 deletions(-)
> 
> diff --git a/hurd/dtable.c b/hurd/dtable.c
> index 5dd04131..41f4d7af 100644
> --- a/hurd/dtable.c
> +++ b/hurd/dtable.c
> @@ -60,18 +60,50 @@ init_dtable (void)
>  	_hurd_dtable[i] = NULL;
>        else
>  	{
> +	  int copy;
>  	  /* Allocate a new file descriptor structure.  */
>  	  struct hurd_fd *new = malloc (sizeof (struct hurd_fd));
>  	  if (new == NULL)
>  	    __libc_fatal ("hurd: Can't allocate initial file descriptors\n");
>  
> -	  /* Initialize the port cells.  */
> -	  _hurd_port_init (&new->port, MACH_PORT_NULL);
> -	  _hurd_port_init (&new->ctty, MACH_PORT_NULL);
> -
> -	  /* Install the port in the descriptor.
> -	     This sets up all the ctty magic.  */
> -	  _hurd_port2fd (new, _hurd_init_dtable[i], 0);
> +	  /* See if this file descriptor is the same as a previous one we have
> +	     already installed.  In this case, we can just copy over the same
> +	     ctty port without making any more RPCs.  We only check the the
> +	     immediately preceding fd and fd 0 -- this should be enough to
> +	     handle the common cases while not requiring quadratic
> +	     complexity.  */
> +	  if (i > 0 && _hurd_init_dtable[i] == _hurd_init_dtable[i - 1])
> +	    copy = i - 1;
> +	  else if (i > 0 && _hurd_init_dtable[i] == _hurd_init_dtable[0])
> +	    copy = 0;
> +	  else
> +	    copy = -1;
> +
> +	  if (copy < 0)
> +	    {
> +	      /* Initialize the port cells.  */
> +	      _hurd_port_init (&new->port, MACH_PORT_NULL);
> +	      _hurd_port_init (&new->ctty, MACH_PORT_NULL);
> +
> +	      /* Install the port in the descriptor.
> +	         This sets up all the ctty magic.  */
> +	      _hurd_port2fd (new, _hurd_init_dtable[i], 0);
> +	    }
> +	  else
> +	    {
> +	      /* Copy over ctty from the already set up file descriptor that
> +	         contains the same port.  We can access the contents of the
> +	         cell without any locking since no one could have seen it
> +	         yet.  */
> +	      mach_port_t ctty = _hurd_dtable[copy]->ctty.port;
> +
> +	      if (MACH_PORT_VALID (ctty))
> +	        __mach_port_mod_refs (__mach_task_self (), ctty,
> +	                              MACH_PORT_RIGHT_SEND, +1);
> +
> +	      _hurd_port_init (&new->port, _hurd_init_dtable[i]);
> +	      _hurd_port_init (&new->ctty, ctty);
> +	    }
>  
>  	  _hurd_dtable[i] = new;
>  	}
> -- 
> 2.39.2
> 

-- 
Samuel
---
Pour une évaluation indépendante, transparente et rigoureuse !
Je soutiens la Commission d'Évaluation de l'Inria.