Hello,
On Tue, Jul 4, 2023 at 9:36 AM David Rowley wrote:
> I've now pushed the patch.
Thanks for the commit!
--
Best regards,
Yuya Watari
On Mon, 3 Jul 2023 at 18:10, Yuya Watari wrote:
> Thank you for your reply. I am +1 to your change. I think these
> assertions will help someone who changes the Bitmapset implementations
> in the future.
I've now pushed the patch.
Thanks for all your reviews and detailed benchmarks.
David
Hello,
On Mon, Jul 3, 2023 at 9:10 AM David Rowley wrote:
> Here's the patch which includes those Asserts. I also made some small
> tweaks to a comment.
Thank you for your reply. I am +1 to your change. I think these
assertions will help someone who changes the Bitmapset implementations
in the
On Mon, 3 Jul 2023 at 09:27, David Rowley wrote:
> If nobody else wants to take a look, then I plan to push the v4 + the
> asserts in the next day or so.
Here's the patch which includes those Asserts. I also made some small
tweaks to a comment.
I understand that Tom thought that the Asserts
Hello,
Thank you for your reply and for creating a new patch.
On Wed, Jun 28, 2023 at 7:58 PM David Rowley wrote:
> Linux with AMD 3990x, again using the patch from [1] with make installcheck
>
> master: 1.41720145 seconds
> v4: 1.392969606 seconds (1.74% faster than master)
> v4 with 0th word
Thank you for running the profiles.
On Tue, 27 Jun 2023 at 21:11, Yuya Watari wrote:
> On Sat, Jun 24, 2023 at 1:15 PM David Rowley wrote:
> > I think it's also important to check we don't slow anything down for
> > more normal-sized sets. The vast majority of sets will contain just a
> >
On Thu, 22 Jun 2023 at 20:59, Yuya Watari wrote:
> Table 1: Planning time and its speedup of Join Order Benchmark
> (n: the number of partitions of each table)
> (Speedup: higher is better)
> 64 | 115.7%
> 128 | 142.9%
> 256 | 187.7%
Thanks for benchmarking. It certainly
On Sat, 24 Jun 2023 at 07:43, Ranier Vilela wrote:
> I worked a bit more on the v4 version and made a new v6 version, with some
> changes.
> I can see some improvement, would you mind testing v6 and reporting back?
Please don't bother. I've already mentioned that I'm not going to
consider any
Em qui., 22 de jun. de 2023 às 05:50, Yuya Watari
escreveu:
> Hello,
>
> On Thu, Jun 22, 2023 at 1:43 PM David Rowley wrote:
> > > 3. Avoid enlargement when nwords is equal wordnum.
> > > Can save cycles when in corner cases?
> >
> > No, you're just introducing a bug here. Arrays in C are
Em qui., 22 de jun. de 2023 às 01:43, David Rowley
escreveu:
> On Thu, 22 Jun 2023 at 00:16, Ranier Vilela wrote:
> > 2. Only compute BITNUM when necessary.
>
> I doubt this will help. The % 64 done by BITNUM will be transformed
> to an AND operation by the compiler which is likely going to be
Hello,
On Tue, Jun 20, 2023 at 1:17 PM David Rowley wrote:
> I've adjusted the attached patch to do that.
Thank you for updating the patch. The v4 patch looks good to me.
I ran another experiment. In the experiment, I issued queries of the
Join Order Benchmark [1] and measured its planning
Hello,
On Thu, Jun 22, 2023 at 1:43 PM David Rowley wrote:
> > 3. Avoid enlargement when nwords is equal wordnum.
> > Can save cycles when in corner cases?
>
> No, you're just introducing a bug here. Arrays in C are zero-based,
> so "wordnum >= a->nwords" is exactly the correct way to
On Thu, 22 Jun 2023 at 00:16, Ranier Vilela wrote:
> 2. Only compute BITNUM when necessary.
I doubt this will help. The % 64 done by BITNUM will be transformed
to an AND operation by the compiler which is likely going to be single
instruction latency on most CPUs which probably amounts to it
Hi,
David Rowley wrote:
>I've adjusted the attached patch to do that.
I think that was room for more improvements.
1. bms_member_index Bitmapset can be const.
2. Only compute BITNUM when necessary.
3. Avoid enlargement when nwords is equal wordnum.
Can save cycles when in corner cases?
On Thu, 15 Jun 2023 at 20:57, Yuya Watari wrote:
>
> On Tue, Jun 13, 2023 at 8:07 PM David Rowley wrote:
> > For the fix in the 0004 patch, I think we can do what you did more
> > simply. I don't think there's any need to perform the loop to find
> > the last non-zero word. We're only deleting
Hello,
On Tue, Jun 13, 2023 at 8:07 PM David Rowley wrote:
> I've incorporated fixes for the bugs in the attached patch. I didn't
> quite use the same approach as you did. I did the fix for 0003
> slightly differently and added two separate paths. We've no need to
> track the last non-zero
On Tue, 13 Jun 2023 at 00:32, Yuya Watari wrote:
> In March, I reported that David's patch caused a degradation in
> planning performance. I have investigated this issue further and found
> some bugs in the patch. Due to these bugs, Bitmapset operations in the
> original patch computed incorrect
Hello,
On Thu, Mar 16, 2023 at 10:30 AM Yuya Watari wrote:
> My idea is to compute the bitwise OR of all bitmapwords of the result
> Bitmapset. The bitwise OR can be represented as a single operation in
> the machine code and does not require any conditional branches. If the
> bitwise ORed value
Hello,
On Tue, Mar 7, 2023 at 1:07 PM David Rowley wrote:
> I adjusted the patch to remove the invariant checks and fixed up a
> couple of things I'd missed. The 0002 patch changes the for loops
> into do while loops. I wanted to see if we could see any performance
> gains from doing this.
I
On Sat, 4 Mar 2023 at 11:08, Tom Lane wrote:
>
> David Rowley writes:
> > On Fri, 3 Mar 2023 at 15:17, Tom Lane wrote:
> > It's true that the majority of Bitmapsets are going to be just 1 word,
> > but it's important to acknowledge that we do suffer in some more
> > extreme cases when
David Rowley writes:
> On Fri, 3 Mar 2023 at 15:17, Tom Lane wrote:
>> Another point here is that I'm pretty sure that just about all
>> bitmapsets we deal with are only one or two words, so I'm not
>> convinced you're going to get any performance win to justify
>> the added management overhead.
On Fri, 3 Mar 2023 at 15:17, Tom Lane wrote:
> (Is it worth carrying both "allocated words" and "nonzero words"
> fields to avoid useless memory-management effort? Dunno.)
It would have been a more appealing thing to do before Bitmapset
became a node type as we'd have had empty space in the
David Rowley writes:
> I suggest tightening the rule even further so instead of just empty
> sets having to be represented as NULL, the rule should be that sets
> should never contain any trailing zero words, which is effectively a
> superset of the "empty is NULL" rule that you've just changed.
On Wed, 1 Mar 2023 at 10:59, Tom Lane wrote:
> When I designed the Bitmapset module, I set things up so that an empty
> Bitmapset could be represented either by a NULL pointer, or by an
> allocated object all of whose bits are zero. I've recently come to
> the conclusion that that was a bad idea
Richard Guo writes:
> It seems that the Bitmapset checked by bms_is_empty_internal cannot be
> NULL from how it is computed by a function. So I wonder if we can
> remove the check of 'a' being NULL in that function, or reduce it to an
> Assert.
Yeah, I think just removing it is sufficient. The
On Thu, Mar 2, 2023 at 6:59 AM Tom Lane wrote:
> Yeah. I split out those executor fixes as 0002; 0003 is the changes
> to bitmapsets proper, and then 0004 removes now-dead code.
+1 to all these patches. Some minor comments from me.
*0003
It seems that the Bitmapset checked by
Nathan Bossart writes:
> On Wed, Mar 01, 2023 at 05:59:45PM -0500, Tom Lane wrote:
>> + /* attidx is zero-based, attrnum is the normal attribute number */
>> + int attrnum = attidx + FirstLowInvalidHeapAttributeNumber;
> nitpick: Shouldn't this be an AttrNumber?
I stuck with the
On Wed, Mar 01, 2023 at 05:59:45PM -0500, Tom Lane wrote:
> bms_first_member is definitely legacy code, so let's just get
> rid of it. Done like that in 0001 below. (This was slightly more
> complex than I foresaw, because some of the callers were modifying
> the result variables. But they're
Nathan Bossart writes:
> On Tue, Feb 28, 2023 at 04:59:48PM -0500, Tom Lane wrote:
>> When I designed the Bitmapset module, I set things up so that an empty
>> Bitmapset could be represented either by a NULL pointer, or by an
>> allocated object all of whose bits are zero. I've recently come to
On Tue, Feb 28, 2023 at 1:59 PM Tom Lane wrote:
> I also discovered that nodeAppend.c is relying on bms_del_members
> not reducing a non-empty set to NULL, because it uses the nullness
> of appendstate->as_valid_subplans as a state boolean.
I seem to recall that Deep and I tripped into this
On Tue, Feb 28, 2023 at 04:59:48PM -0500, Tom Lane wrote:
> When I designed the Bitmapset module, I set things up so that an empty
> Bitmapset could be represented either by a NULL pointer, or by an
> allocated object all of whose bits are zero. I've recently come to
> the conclusion that that
When I designed the Bitmapset module, I set things up so that an empty
Bitmapset could be represented either by a NULL pointer, or by an
allocated object all of whose bits are zero. I've recently come to
the conclusion that that was a bad idea and we should instead have
a convention like the
32 matches
Mail list logo
