Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
From:   "NeilBrown" <neilb@suse.de>
To:     "Chuck Lever" <cel@kernel.org>
Cc:     linux-nfs@vger.kernel.org, "Chuck Lever" <chuck.lever@oracle.com>
Subject: Re: [PATCH RFC 0/4] Encode NFSv4 attributes via a branch table
In-reply-to: <168808788945.7728.6965361432016501208.stgit@manet.1015granger.net>
References: <168808788945.7728.6965361432016501208.stgit@manet.1015granger.net>
Date:   Mon, 03 Jul 2023 14:48:07 +1000
Message-id: <168835968730.8939.17203263812842647260@noble.neil.brown.name>
Precedence: bulk

On Fri, 30 Jun 2023, Chuck Lever wrote:
> Here's something just for fun. I've converted nfsd4_encode_fattr4()
> to use a bitmask loop, calling an encode helper for each attribute
> to be encoded. Rotten tomatoes and gold stars are both acceptible.

Tomatoes or stars .... it is a hard choice :-)

I wonder what the compiler does with this code.
If it unrolls the loop and inlines the functions - which it probably can
do as the array of pointers is declared const - you end up with much the
same result as the current code.

And I wonder where the compiler puts the code in each conditional now.
If it assumes an if() is unlikely, then it would all be out-of-line
which sounds like part of your goal (or maybe just a nice-to-have).

If the compiler does, or can be convinced to, do the unroll and inline
and unlikely optimisations, then I think I'd give this a goal star.

I guess in practice some of the attributes are "likely" and many are
"unlikely".  With the current code we could easily annotate that if we
wanted to and thought (or measured) there was any value.  With the
looping code we cannot really annotate the likelihood of each.

The code-generation idea is intriguing.  Even if we didn't reach that
goal, having the code highly structured as though it were auto-generated
would be no bad thing.

Thanks,
NeilBrown