On Sun, Apr 19, 2020 at 05:29:52PM -0300, Ranier Vilela wrote:
>Em dom., 19 de abr. de 2020 às 16:33, Tomas Vondra <
>tomas.vondra@2ndquadrant.com> escreveu:
>
>> On Sun, Apr 19, 2020 at 11:24:38AM -0300, Ranier Vilela wrote:
>> >Hi,
>> >strlen it is one of the low fruits that can be harvested.
>> >What is your opinion?
>> >
>>
>> That assumes this actually affects/improves performance, without any
>> measurements proving that. Considering large number of the places you
>> modified are related to DDL (CreateComment, ChooseIndexColumnNames, ...)
>> or stuff that runs only once or infrequently (like the changes in
>> PostmasterMain or libpqrcv_get_senderinfo). Likewise, it seems entirely
>> pointless to worry about strlen() overhead e.g. in fsync_parent_path
>> which is probably dominated by I/O.
>>
>With code as interconnected as postgres, it is difficult to say that a
>function, which calls strlen, repeatedly, would not have any gain.
>Regarding the functions, I was just being consistent, trying to remove all
>occurrences, even where, there is very little gain.
>
That very much depends on the function, I think. For most places modified
by this patch it's not that hard, I think. The DDL cases (comments and
indexes) seem pretty clear. Similarly for the command parsing, wal
receiver, lockfile creation, guc, exec.c, and so on.
Perhaps the only places worth changing might be xml.c and spell.c, but
I'm not convinced even these are worth it, really.
>
>>
>> Maybe there are places where this would help, but I don't see a reason
>> to just throw away all strlen calls and replace them with something
>> clearly less convenient and possibly more error-prone (I'd expect quite
>> a few off-by-one mistakes with this).
>>
>Yes, always, it is prone to errors, but for the most part, they are safe
>changes.
>It passes all 199 tests, of course it has not been tested in a real
>production environment.
>Perhaps the time is not the best, the end of the cycle, but, once done, I
>believe it would be a good harvest.
>
I wasn't really worried about bugs in this patch, but rather in future
changes made to this code. Off-by-one errors are trivial to make.
regards
--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
