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, <jaillet...@apache.org
>> <mailto:jaillet...@apache.org>> 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
