From: | Andrew Dunstan <andrew(at)dunslane(dot)net> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | Robert Haas <robertmhaas(at)gmail(dot)com>, Peter Eisentraut <peter_e(at)gmx(dot)net>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>, Nikolay Samokhvalov <samokhvalov(at)gmail(dot)com> |
Subject: | Re: xpath processing brain dead |
Date: | 2009-02-27 22:29:28 |
Message-ID: | 49A86948.8080100@dunslane.net |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Tom Lane wrote:
> Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
>
>>> I'll do some tests to see what the cost of extra xml parsing might be.
>>>
>
>
>> The extra cost appears to be fairly negligible.
>>
>
> Uh, you didn't actually show a comparison of before and after?
> What it looks like to me is that this approach is free or nearly so for
> well-formed documents, but doubles the parsing cost for forests.
> Which is likely to annoy anyone who's really depending on the
> capability.
>
The difference is lost in the noise.
Without fix:
Time: 24.619 ms
Time: 24.245 ms
Time: 25.179 ms
With fix:
Time: 24.084 ms
Time: 21.980 ms
Time: 23.765 ms
The test is done on 10,000 short fragments each parsed 10 times (or 20
times with the fix).
I'll test again on some longer fragments since you don't seem convinced.
> Also,
>
>
>> ! if (*VARDATA(xpath_expr_text) == '/')
>>
>
> This is risking a core dump if the xpath expr is of zero length. You
> need something like
>
> if (xpath_len > 0 && *VARDATA(xpath_expr_text) == '/')
>
OK.
> It would also be a good idea if the allocation of string and xpath_expr
> had a comment about why it's allocating extra space (something like "see
> hacks below for use of this extra space" would be sufficient).
>
OK.
cheers
andrew
From | Date | Subject | |
---|---|---|---|
Next Message | Jaime Casanova | 2009-02-27 22:59:05 | Re: Updates of SE-PostgreSQL 8.4devel patches (r1530) |
Previous Message | Tom Lane | 2009-02-27 22:06:35 | Re: Updates of SE-PostgreSQL 8.4devel patches (r1530) |