Re: svn commit: r1907634 - /apr/apr/trunk/include/apr_buckets.h

Ruediger Pluem <[email protected]>
Newsgroups gmane.comp.apache.apr.devel
Message-ID <[email protected]>

On 2/14/23 7:28 PM, Christophe JAILLET wrote:
> Le 14/02/2023 à 11:51, Ivan Zhakov via dev a écrit :
>> On Tue, 14 Feb 2023 at 10:58, <[email protected] <mailto:[email protected]>> wrote:
>>
>>     Author: jailletc36
>>     Date: Tue Feb 14 07:56:50 2023
>>     New Revision: 1907634
>>
>>     URL: http://svn.apache.org/viewvc?rev=1907634&view=rev
>>     <http://svn.apache.org/viewvc?rev=1907634&view=rev>
>>     Log:
>>     Re-order the fields of 'struct apr_bucket_file' to avoid a hole and
>>     some padding.
>>
>>     Before the patch, pahole states that:
>>
>>     struct apr_bucket_file {
>>              apr_bucket_refcount        refcount;             /*     0         4 */
>>
>>              /* XXX 4 bytes hole, try to pack */
>>
>>              apr_file_t *               fd;                   /*     8         8 */
>>              apr_pool_t *               readpool;             /*    16         8 */
>>              int                        can_mmap;             /*    24         4 */
>>
>>              /* XXX 4 bytes hole, try to pack */
>>
>>              apr_size_t                 read_size;            /*    32         8 */
>>
>>              /* size: 40, cachelines: 1, members: 5 */
>>              /* sum members: 32, holes: 2, sum holes: 8 */
>>              /* last cacheline: 40 bytes */
>>     };
>>
>> Nice, but I'd like to note that this change breaks ABI and can be released only in APR 2.0.
>>
> 
> Sure.
> 
> I was just wondering if such small optimizations make sense or not.
> It potentially makes backport of some other patches more difficult and breaks ABI.

This is true, but I see only a low risk of making backports difficult for structures that seem to be set and haven't been touched
for a long time. And even if we need to touch them we can still backport. It is just more work then.

> 
> So, does it worth the effort?

I would say yes at least for structures that seem to be set and stable.

> 
> I've spotted a few of them in APR, and would make sure that such patches are welcomed before committing other things.

As always, really appreciate your look on these gory details.

Regards

Rüdiger
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.