Marton Balint: > > > On Wed, 6 Jan 2021, Andreas Rheinhardt wrote: > >> When the difference of the longest size and the average size of >> collection of strings is smaller than the size of a pointer, it makes >> sense to store the strings directly in an array instead of using an >> array of pointers to strings (unless doing so precludes deduplicating >> strings); doing so also avoids relocations. This requirement is >> fulfilled for several arrays of pointers to strings that are changed in >> this commit. >> >> Signed-off-by: Andreas Rheinhardt <andreas.rheinha...@gmail.com> >> --- >> Now used sizeof("longest string") for the array element size. > > Duplicating the "longest string" is very ugly, I don't find it better > than a constant number, because a sizeof still does not say it tries to > find the longest string. > > TBH I am not a fan of these kind of patches for a few byte of savings, > and when changing the list of strings it can actually become worse which > won't be checked by anybody... > When using position independent code, each of these pointers that is different from NULL requires a relocation (on ELF, these are quite expensive: 24 byte when using 64 byte systems), so that these arrays of pointers can't be in .rodata, but only in .data (or .data.rel.ro). Even worse, these static const objects aren't initialized in a lazy manner,* so that pages containing pointers to be relocated are automatically dirty even when they are actually never used. I think it is of the utmost importance to counter this and this patch does so in a few instances. Of course I am all ears for how to make it clear that someone who modifies the strings also needs to check the array dimensions.
(Here is a snippet for those who want to test this for themselves: +typedef struct Entry { + const char *str; + char array[4088]; +} Entry; + +#define REPEAT(...) __VA_ARGS__, __VA_ARGS__, __VA_ARGS__, __VA_ARGS__ +#define REPEAT2(...) REPEAT(REPEAT(REPEAT(REPEAT(REPEAT(REPEAT(REPEAT(__VA_ARGS__))))))) + +const Entry unused[16384] = { REPEAT2({ "", "" }) }; + ) - Andreas *: It seems that Windows does better: https://devblogs.microsoft.com/oldnewthing/20160413-00/?p=93301 _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".