Date: Fri, 06 Sep 2024 15:02:37 +0900 (JST)
Message-Id: <20240906.150237.57111972562260411.ishii@postgresql.org>
To: dgrowleyml@gmail.com
Cc: ashutosh.bapat.oss@gmail.com, pgsql-hackers@postgresql.org
Subject: Re: Add memory/disk usage for WindowAgg nodes in EXPLAIN
From: Tatsuo Ishii <ishii@postgresql.org>
In-Reply-To: 
 <CAApHDvqgBcwV8iVZY0f1+D2TQrRAbjFA5AmkX-BHL2z9PhTQSQ@mail.gmail.com>
References: <20240905.143809.436581948007856565.ishii@postgresql.org>
	<20240906.132131.1196176059464159407.ishii@postgresql.org>
	<CAApHDvqgBcwV8iVZY0f1+D2TQrRAbjFA5AmkX-BHL2z9PhTQSQ@mail.gmail.com>
Mime-Version: 1.0
Content-Type: Text/Plain; charset=us-ascii
Content-Transfer-Encoding: 7bit
Archived-At: 
 <https://www.postgresql.org/message-id/20240906.150237.57111972562260411.ishii%40postgresql.org>
Precedence: bulk

> Thanks for making the adjustments to this.
> 
> I don't think there is any need to call tuplestore_updatemax() from
> within writetup_heap(). That means having to update the maximum space
> used every time a tuple is written to disk. That's a fairly massive
> overhead.
> 
> Instead, it should be fine to modify tuplestore_updatemax() to set a
> flag to true if state->status != TSS_INMEM and then record the disk
> space used. That flag won't ever be set to false again.
> tuplestore_storage_type_name() should just return "Disk" if the new
> disk flag is set, even if state->status == TSS_INMEM. Since the
> work_mem size won't change between tuplestore_clear() calls, if we've
> once spilt to disk, then we shouldn't care about the memory used for
> runs that didn't. Those will always have used less memory.
> 
> I did this quickly, but playing around with the attached, I didn't see
> any slowdown.

Your patch looks good to me and I confirmed that with your patch I
didn't see any slowdown either. Thanks!

Best reagards,
--
Tatsuo Ishii
SRA OSS K.K.
English: http://www.sraoss.co.jp/index_en/
Japanese:http://www.sraoss.co.jp