> On Sun, Aug 11, 2024 at 09:34:55PM GMT, Dmitry Dolgov wrote:
> > On Sun, Aug 11, 2024 at 07:54:05PM +0300, Sergei Kornilov wrote:
> >
> > This feature will improve my monitoring. Even in patch 0001. I think there are many other people in the thread who
thinkthis is useful. So maybe we should move it forward? Any complaints about the overall design? I see in the
discussionit was mentioned that it would be good to measure performance difference.
> >
> > PS: patch cannot be applied at this time, it needs another rebase.
>
> Yeah, it seems like most people are fine with the first patch and the
> simplest approach. I'm going to post a rebased version and a short
> thread summary soon.
Ok, here is the rebased version. If anyone would like to review them, below is
the short summary of the thread. Currently the patch series contains 4 changes:
* 0001-Prevent-jumbling-of-every-element-in-ArrayExpr.patch
Implements the simplest way to handle constant arrays, if the array contains
only constants it will be reduced. This is the basis, if I read it correctly
Nathan and Michael expressed that they're mostly fine with this one.
Michael seems to be skeptical about the "merged" flag in the LocationLen
struct, but from what I see the proposed alternative has problems as well.
There was also a note that the loop over constants has to be benchmarked, but
it's not entirely clear for me in which dimentions to benchmark (i.e. what
are the expectations). For both I'm waiting on a reply to my questions.
* 0002-Reusable-decimalLength-functions.patch
A small refactoring to make already existing "powers" functonality reusable
for following patches.
* 0003-Merge-constants-in-ArrayExpr-into-groups.patch
Makes handling of constant arrays smarter by taking into account number of
elements in the array. This way records are merged into groups power of 10,
i.e. arrays with length 55 will land in a group 10-99, with lenght 555 in a
group 100-999 etc. This was proposed by Alvaro, and personally I like this
approach, because it remediates the issue of one-size-fits-all for the static
threshold. But there are opinions that this introduces too much complexity.
* 0004-Introduce-query_id_const_merge_threshold.patch
Fine tuning for the previous patch, makes only arrays with the length over a
certain threshold to be reduced.
On top of that Yasuo Honda and Jakub Wartak have provided a couple of practical
examples, where handling of constant arrays has to be improved. David Geier
pointed out some examples that might be confusing as well. All those are
definitely worth addressing, but out of scope of this patch for now.
