On Thu, Jun 06, 2013 at 07:02:27PM +0530, Amit Kapila wrote:
> On Tuesday, June 04, 2013 12:37 AM Noah Misch wrote:
> This patch can give good performance gain in the scenario described by you.
> Infact I had taken the readings with patch, it shows similar gain.
Thanks for testing.
> This patch would increase the relcache size, as for each constraint on table
> it would increase 4 bytes irrespective of whether that can give performance
> benefit or not.
Yep, sizeof(Node *) bytes.
> Why in function CheckConstraintFetch(), the node is not formed from string?
That's to avoid the cost of stringToNode() if the field is not needed during
the life of the cache entry.
> > Some call sites need to modify the node tree, so the patch has them do
> > copyObject(). I ran a microbenchmark of copyObject() on the cached
> > node tree vs. redoing stringToNode(), and copyObject() still won by a
> > factor of four.
>
> I have not tried any performance run to measure if extra copyObject() has
> added any benefit.
Callers must not modify the cache entry; those that would otherwise do so must
use copyObject() first. I benchmarked to ensure they wouldn't be better off
ignoring the cached node tree and calling stringToNode() themselves.
> What kind of benchmark you use to validate it?
It boiled down to comparing runtime of loops like these:
while (n-- > 0)
copyObject(RelationGetConstraint(r, 0));
while (n-- > 0)
stringToNode(r->rd_att->constr->check[0].ccbin);
--
Noah Misch
EnterpriseDB http://www.enterprisedb.com
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
