patch: tiny patch to correct stringbuffer size estimate

This patch tweaks the size estimate when escaping strings to count the
enclosing quotes.

-O

These StringBuffer woes remember of an old request we made to this list... The last time I checked driver's sources, all acess to preparedstatement's stringbuffer are synchronized... this isn't necessary as jre's Stringbuffer already has all of its methods synchronized.
The driver is still doing that? Just curious.

On Wed, 23 Jul 2003 01:54:53 +1200
Oliver Jowett <oliver(at)opencloud(dot)com> wrote:

> This patch tweaks the size estimate when escaping strings to count the
> enclosing quotes.
>
> -O
>

--

/~\ The ASCII Felipe Schnack (felipes(at)ritterdosreis(dot)br)
\ / Ribbon Campaign Analista de Sistemas
X Against HTML Cel.: 51-91287530
/ \ Email! Linux Counter #281893

Centro Universitário Ritter dos Reis
http://www.ritterdosreis.br
ritter(at)ritterdosreis(dot)br
Fone: 51-32303341

On Tue, Jul 22, 2003 at 11:20:11AM -0300, Felipe Schnack wrote:

> These StringBuffer woes remember of an old request we made to this
> list... The last time I checked driver's sources, all acess to
> preparedstatement's stringbuffer are synchronized... this isn't
> necessary as jre's Stringbuffer already has all of its methods
> synchronized. The driver is still doing that? Just curious.

Yes, but to be threadsafe with a shared buffer you do need to synchronize as
the driver does a series of operations to accumulate a string that need to
be atomic. Synchronizing on each individual operation isn't enough.

That said, it's pretty silly to write code that depends on a Statement being
threadsafe :)

-O

This is already accounted for in the 10% that is added to it, isn't it?

Oliver Jowett wrote:
> This patch tweaks the size estimate when escaping strings to count the
> enclosing quotes.
>
> -O
>
>
> ------------------------------------------------------------------------
>
> Index: AbstractJdbc1Statement.java
> ===================================================================
> RCS file: /projects/cvsroot/pgsql-server/src/interfaces/jdbc/org/postgresql/jdbc1/AbstractJdbc1Statement.java,v
> retrieving revision 1.28
> diff -u -c -r1.28 AbstractJdbc1Statement.java
> *** AbstractJdbc1Statement.java 22 Jul 2003 05:17:09 -0000 1.28
> --- AbstractJdbc1Statement.java 22 Jul 2003 13:53:03 -0000
> ***************
> *** 1034,1040 ****
> synchronized (sbuf)
> {
> sbuf.setLength(0);
> ! sbuf.ensureCapacity(x.length() + (int)(x.length() / 10));
> sbuf.append('\'');
> escapeString(x, sbuf);
> sbuf.append('\'');
> --- 1034,1040 ----
> synchronized (sbuf)
> {
> sbuf.setLength(0);
> ! sbuf.ensureCapacity(2 + x.length() + (int)(x.length() / 10));
> sbuf.append('\'');
> escapeString(x, sbuf);
> sbuf.append('\'');
>
>
> ------------------------------------------------------------------------
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 4: Don't 'kill -9' the postmaster

--
Fernando Nasser
Red Hat Canada Ltd. E-Mail: fnasser(at)redhat(dot)com
2323 Yonge Street, Suite #300
Toronto, Ontario M4P 2C9

On Tue, Jul 22, 2003 at 10:36:01AM -0400, Fernando Nasser wrote:
> This is already accounted for in the 10% that is added to it, isn't it?

Only if you have a string > 20 characters, due to truncation on integer
division, no?

-O

> Oliver Jowett wrote:
> >This patch tweaks the size estimate when escaping strings to count the
> >enclosing quotes.

> >*** AbstractJdbc1Statement.java 22 Jul 2003 05:17:09 -0000 1.28
> >--- AbstractJdbc1Statement.java 22 Jul 2003 13:53:03 -0000
> >***************
> >*** 1034,1040 ****
> > synchronized (sbuf)
> > {
> > sbuf.setLength(0);
> >! sbuf.ensureCapacity(x.length() +
> >(int)(x.length() / 10));
> > sbuf.append('\'');
> > escapeString(x, sbuf);
> > sbuf.append('\'');
> >--- 1034,1040 ----
> > synchronized (sbuf)
> > {
> > sbuf.setLength(0);
> >! sbuf.ensureCapacity(2 + x.length() +
> >(int)(x.length() / 10));
> > sbuf.append('\'');
> > escapeString(x, sbuf);
> > sbuf.append('\'');

Oliver Jowett wrote:
> On Tue, Jul 22, 2003 at 10:36:01AM -0400, Fernando Nasser wrote:
>
>>This is already accounted for in the 10% that is added to it, isn't it?
>
>
> Only if you have a string > 20 characters, due to truncation on integer
> division, no?
>

Good point, although the doubling of the Buffer pool is irrelevant in
those cases. Any difference is only measurable in the hundreds of K sizes.

Anyway, it will not hurt adding 2.

Regards,
Fernando

>>Oliver Jowett wrote:
>>
>>>This patch tweaks the size estimate when escaping strings to count the
>>>enclosing quotes.
>>
>
>>>*** AbstractJdbc1Statement.java 22 Jul 2003 05:17:09 -0000 1.28
>>>--- AbstractJdbc1Statement.java 22 Jul 2003 13:53:03 -0000
>>>***************
>>>*** 1034,1040 ****
>>> synchronized (sbuf)
>>> {
>>> sbuf.setLength(0);
>>>! sbuf.ensureCapacity(x.length() +
>>>(int)(x.length() / 10));
>>> sbuf.append('\'');
>>> escapeString(x, sbuf);
>>> sbuf.append('\'');
>>>--- 1034,1040 ----
>>> synchronized (sbuf)
>>> {
>>> sbuf.setLength(0);
>>>! sbuf.ensureCapacity(2 + x.length() +
>>>(int)(x.length() / 10));
>>> sbuf.append('\'');
>>> escapeString(x, sbuf);
>>> sbuf.append('\'');
>>
>

--
Fernando Nasser
Red Hat Canada Ltd. E-Mail: fnasser(at)redhat(dot)com
2323 Yonge Street, Suite #300
Toronto, Ontario M4P 2C9

Am I wrong or the jdbc spec say they should be thread-safe?

On Wed, 23 Jul 2003 02:28:07 +1200
Oliver Jowett <oliver(at)opencloud(dot)com> wrote:

> On Tue, Jul 22, 2003 at 11:20:11AM -0300, Felipe Schnack wrote:
>
> > These StringBuffer woes remember of an old request we made to this
> > list... The last time I checked driver's sources, all acess to
> > preparedstatement's stringbuffer are synchronized... this isn't
> > necessary as jre's Stringbuffer already has all of its methods
> > synchronized. The driver is still doing that? Just curious.
>
> Yes, but to be threadsafe with a shared buffer you do need to synchronize as
> the driver does a series of operations to accumulate a string that need to
> be atomic. Synchronizing on each individual operation isn't enough.
>
> That said, it's pretty silly to write code that depends on a Statement being
> threadsafe :)
>
> -O

--

/~\ The ASCII Felipe Schnack (felipes(at)ritterdosreis(dot)br)
\ / Ribbon Campaign Analista de Sistemas
X Against HTML Cel.: 51-91287530
/ \ Email! Linux Counter #281893

Centro Universitário Ritter dos Reis
http://www.ritterdosreis.br
ritter(at)ritterdosreis(dot)br
Fone: 51-32303341

Felipe,

Why do you think the synchronizations are unnecessary?

Consider the following code:

synchronized (sbuf)
{
sbuf.setLength(0);
sbuf.ensureCapacity(x.length() + (int)(x.length() / 10));
sbuf.append('\'');
escapeString(x, sbuf);
sbuf.append('\'');
bind(parameterIndex, sbuf.toString(), type);
}

If two thread where to perform this code at the same time using the same
sbuf the results would be garbage even if the methods on sbuf are all
syncronized. It is the set of actions on sbuf here that needs to be
syncronized, not each individual call. I beleive all the uses of
synchronization in the driver are of a similar usage pattern.

thanks,
--Barry

Felipe Schnack wrote:
> These StringBuffer woes remember of an old request we made to this list... The last time I checked driver's sources, all acess to preparedstatement's stringbuffer are synchronized... this isn't necessary as jre's Stringbuffer already has all of its methods synchronized.
> The driver is still doing that? Just curious.
>
> On Wed, 23 Jul 2003 01:54:53 +1200
> Oliver Jowett <oliver(at)opencloud(dot)com> wrote:
>
>
>>This patch tweaks the size estimate when escaping strings to count the
>>enclosing quotes.
>>
>>-O
>>
>
>
>

Looking at this case I don't think they are unnecessary. But I would like to just raise some questions for clarification
- The JDBC spec doesn't say these objects should be used in a thread-safe way? What I mean: your application/container shouldn't use a single PreparedStatement in multiple threads)... I think I saw this somewhere, but I might be wrong
- Maybe the unnecessary synch'ing is the one stringbuffer does internally?

On Tue, 22 Jul 2003 09:10:00 -0700
Barry Lind <blind(at)xythos(dot)com> wrote:

> Felipe,
>
> Why do you think the synchronizations are unnecessary?
>
> Consider the following code:
>
> synchronized (sbuf)
> {
> sbuf.setLength(0);
> sbuf.ensureCapacity(x.length() + (int)(x.length() / 10));
> sbuf.append('\'');
> escapeString(x, sbuf);
> sbuf.append('\'');
> bind(parameterIndex, sbuf.toString(), type);
> }
>
> If two thread where to perform this code at the same time using the same
> sbuf the results would be garbage even if the methods on sbuf are all
> syncronized. It is the set of actions on sbuf here that needs to be
> syncronized, not each individual call. I beleive all the uses of
> synchronization in the driver are of a similar usage pattern.
>
> thanks,
> --Barry
>
> Felipe Schnack wrote:
> > These StringBuffer woes remember of an old request we made to this list... The last time I checked driver's sources, all acess to preparedstatement's stringbuffer are synchronized... this isn't necessary as jre's Stringbuffer already has all of its methods synchronized.
> > The driver is still doing that? Just curious.
> >
> > On Wed, 23 Jul 2003 01:54:53 +1200
> > Oliver Jowett <oliver(at)opencloud(dot)com> wrote:
> >
> >
> >>This patch tweaks the size estimate when escaping strings to count the
> >>enclosing quotes.
> >>
> >>-O
> >>
> >
> >
> >
>
>
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 6: Have you searched our list archives?
>
> http://archives.postgresql.org

--

/~\ The ASCII Felipe Schnack (felipes(at)ritterdosreis(dot)br)
\ / Ribbon Campaign Analista de Sistemas
X Against HTML Cel.: 51-91287530
/ \ Email! Linux Counter #281893

Centro Universitário Ritter dos Reis
http://www.ritterdosreis.br
ritter(at)ritterdosreis(dot)br
Fone: 51-32303341

On 22/07/2003 17:36 Felipe Schnack wrote:
> Looking at this case I don't think they are unnecessary. But I would
> like to just raise some questions for clarification
> - The JDBC spec doesn't say these objects should be used in a
> thread-safe way? What I mean: your application/container shouldn't use a
> single PreparedStatement in multiple threads)... I think I saw this
> somewhere, but I might be wrong
> - Maybe the unnecessary synch'ing is the one stringbuffer does
> internally?
>

Some databases might not even like sharing a connection between threads (I
know is is true for ODBC implemenations) so best practice would never see
synch'ing to be neccessary. OTOH, synchronization does not cause much of a
performance hit on modern JVMs (according to developer works).
--
Paul Thomas
+------------------------------+---------------------------------------------+
| Thomas Micro Systems Limited | Software Solutions for the Smaller
Business |
| Computer Consultants |
http://www.thomas-micro-systems-ltd.co.uk |
+------------------------------+---------------------------------------------+

Patch applied.

--Barry

Oliver Jowett wrote:
> This patch tweaks the size estimate when escaping strings to count the
> enclosing quotes.
>
> -O
>
>
> ------------------------------------------------------------------------
>
> Index: AbstractJdbc1Statement.java
> ===================================================================
> RCS file: /projects/cvsroot/pgsql-server/src/interfaces/jdbc/org/postgresql/jdbc1/AbstractJdbc1Statement.java,v
> retrieving revision 1.28
> diff -u -c -r1.28 AbstractJdbc1Statement.java
> *** AbstractJdbc1Statement.java 22 Jul 2003 05:17:09 -0000 1.28
> --- AbstractJdbc1Statement.java 22 Jul 2003 13:53:03 -0000
> ***************
> *** 1034,1040 ****
> synchronized (sbuf)
> {
> sbuf.setLength(0);
> ! sbuf.ensureCapacity(x.length() + (int)(x.length() / 10));
> sbuf.append('\'');
> escapeString(x, sbuf);
> sbuf.append('\'');
> --- 1034,1040 ----
> synchronized (sbuf)
> {
> sbuf.setLength(0);
> ! sbuf.ensureCapacity(2 + x.length() + (int)(x.length() / 10));
> sbuf.append('\'');
> escapeString(x, sbuf);
> sbuf.append('\'');
>
>
> ------------------------------------------------------------------------
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 4: Don't 'kill -9' the postmaster