Storing possibly-arrays as arrays

[Alejandro Colomar (man-pages)]

On 12/18/21 20:55, Alejandro Colomar (man-pages) wrote:
> Hi Valentin,
> 
> While implementing the "index" option, I found this code, which didn't 
> convince me:
> 
> [
>      shr_is_array = (nxt_conf_type(acf->share) == NXT_CONF_ARRAY);
>      conf->nshares = shr_is_array ? 
> nxt_conf_array_elements_count(acf->share)
>                                   : 1;
>      conf->shares = nxt_mp_zget(mp, sizeof(nxt_http_static_share_t)
>                                     * conf->nshares);
>      if (nxt_slow_path(conf->shares == NULL)) {
>          return NXT_ERROR;
>      }
> 
>      if (shr_is_array) {
>          for (i = 0; i < conf->nshares; i++) {
>              cv = nxt_conf_get_array_element(acf->share, i);
>              nxt_conf_get_string(cv, &str);
> 
>              var = nxt_var_compile(&str, mp, 1);
>              if (nxt_slow_path(var == NULL)) {
>                  return NXT_ERROR;
>              }
> 
>              conf->shares[i].var = var;
>              conf->shares[i].is_const = nxt_var_is_const(var);
>          }
> 
>      } else {
>          nxt_conf_get_string(acf->share, &str);
> 
>          var = nxt_var_compile(&str, mp, 1);
>          if (nxt_slow_path(var == NULL)) {
>              return NXT_ERROR;
>          }
> 
>          conf->shares[0].var = var;
>          conf->shares[0].is_const = nxt_var_is_const(var);
>      }
> ]
> 
> Most of the code is duplicated for the cases that 'share is not an 
> array' and 'share is an array', changing just a single line.  How about 
> reorganizing the data structures so that when both a string/object and 
> an array are a valid type
>      .type = NXT_CONF_VLDT_* | NXT_CONF_VLDT_ARRAY,
> the data is _always_ stored as an array (of size 1 if it was not an 
> array originally)?
> 
> It could allow to simplify the code above to the following one:
> 
> [
>      conf->nshares = nxt_conf_array_elements_count(acf->share);
>      conf->shares = nxt_mp_zget(mp, sizeof(nxt_http_static_share_t)
>                                     * conf->nshares);
>      if (nxt_slow_path(conf->shares == NULL)) {
>          return NXT_ERROR;
>      }
> 
>      for (i = 0; i < conf->nshares; i++) {
>          cv = nxt_conf_get_array_element(acf->share, i);
>          nxt_conf_get_string(cv, &str);
> 
>          var = nxt_var_compile(&str, mp, 1);
>          if (nxt_slow_path(var == NULL)) {
>              return NXT_ERROR;
>          }
> 
>          conf->shares[i].var = var;
>          conf->shares[i].is_const = nxt_var_is_const(var);
>      }
> ]
> 
> Things to consider would be:
> - Does any code behave differently for arrays and for strings/objects?
> - Does forcing strings/objects as arrays impose non-negligible 
> performance penalties?
> 
> Another alternative, but maybe not so good since it kind of defeats the 
> difference between (nxt) arrays and non-arrays, would be to modify 
> nxt_conf_array_elements_count() so that

Oops, I meant nxt_conf_get_array_element().

>      if (value->type != NXT_CONF_VALUE_ARRAY) {
>          return (index == 0) ? value : NULL;
>      }
> This would be similar to C pointers to variables, which can always be 
> considered as pointers to the first element of an array of size 1.
> 
> 
> Cheers,
> Alex
> 
> 

-- 
Alejandro Colomar
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/