> On 7 Feb 2025, at 06:54, Daniil Davydov <3danissimo@gmail.com> wrote:
>
> On Thu, Feb 6, 2025 at 8:23 PM Daniel Gustafsson <daniel@yesql.se> wrote:
>>
>> You should include the configure.ac changes and not just the generated code in
>> configure (which is fine to include for review, but the committer will
>> re-generate it regardless). Please also include the corresponding Meson change
>> to keep the buildsystems in sync.
>
> Thank you for your comment. I'm applying a patch that also includes
> configure.ac, meson_options.txt and meson.build changes.
+AC_DEFINE_UNQUOTED([SLRU_PAGES_PER_SEGMENT], ${slru_pages_per_segment}, [
+ SLRU segment size in pages. A page is the same BLCKSZ as is used everywhere
+ else in Postgres. The segment size can be chosen somewhat arbitrarily;
+ ...
It's not per project style to add large paragraphs like that to autoconf or
Meson, which segways nicely into commenting that the GUC and the build-time
parameters are missing from the documentation. Something like the above
message should be in the documentation rather than the build infrastructure.
- elog(NOTICE, "Called SlruSyncFileTag() for segment %lld on path %s",
- (long long) ftag.segno, path);
-
...
- elog(NOTICE, "Called SlruDeleteSegment() for segment %lld",
- (long long) ftag.segno);
-
What's the motivation for these changes?
> It would also be interesting to know what you think about the idea of
> allowing such a parameter to be configured.
It could perhaps be useful for (mostly) testing purposes, but since it requires
recompilation why not just change SLRU_PAGES_PER_SEGMENT in slru.h and
recompile? That would save a lot of code while achieving the same thing no?
If you foresee a performance enchancement in production usecases at certaint
values I recommend doing benchmarking to show this.
--
Daniel Gustafsson
