You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@velocity.apache.org by Serge Knystautas <sk...@gmail.com> on 2006/03/02 05:01:24 UTC

SimplePool replacement

I did some memory profiling, and for each runtime instance, by default
it creates 20 parsers.  According to yourkit, each parser uses just
over 100kb heap space, so each runtime instance is costing me 2mb heap
space.

I can simply reduce the # of parsers, but the problem is that simple
pool is a very, very primitive pool implementation... it holds a
constant number parsers and if you exceed, it simply creates and
disposes a parser (ballpark of 1ms per parser instantiation in my
benchmarks).

I'd like to convert simplepool to an interface and then make the
current behavior an impl so that you could swap in something like
commons pool.  Is there interest in such a patch?

--
Serge Knystautas
Lokitech >> software . strategy . design >> http://www.lokitech.com
p. 301.656.5501
e. sergek@lokitech.com

---------------------------------------------------------------------
To unsubscribe, e-mail: velocity-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: velocity-dev-help@jakarta.apache.org


Re: SimplePool replacement

Posted by Will Glass-Husain <wg...@forio.com>.
Ah, thanks -- actual data is always useful.

That seems like the right approach - define a SimplePool style interface and
a factory interface.  Then let the user configure a SimplePoolFactory
plugin.

WILL


On 3/3/06, Serge Knystautas <sk...@gmail.com> wrote:
>
> On 3/3/06, Will Glass-Husain <wg...@forio.com> wrote:
> > Hi,
> >
> > I'd argue that SimplePool is a utility class and not part of the public
> API
> > (so it's ok to change this).  Have to draw the line somewhere -
> otherwise we
> > can never change anything.  Anyone object?
> >
> > I'm not really following the problem with factories -- I'll wait to see
> > until I see some code to comment on this.
> >
> > Llewellyn Falco has a Eclipse code formatting file.  I had a copy but my
> > computer is in the shop. Llewellyn are you out there?
> >
> > Best, WILL
>
> I reran my benchmarks to conclude whether parser pooling is necessary.
> I changed to 0 parsers in the pool (so a new one is created each
> template compilation) for these numbers...
>
> 4k template file
> 3.4Ghz HP laptop
> Win JDK 1.5
> run 1000 times
>
> 5,250ms to compile template by reusing parser
> 6,953ms to compile template creating a parser each time
>
> So given 1,000 iterations, about 1.7ms to create a parser.
>
> WRT my comments about refactoring, the SimplePool is IMO not a pool
> but an array.  Right now, RuntimeInstance creates simplepool, iterates
> through the # of spots in the pool, constructs a parser and assigns it
> to that spot.  Then whenever RuntimeInstance needs a parser, it checks
> if one available in simplepool, and if not, it constructs a new one.
>
> I think of a pool as providing object instances and the user does not
> know whether it is a reused or new instance.  To do this, a pool needs
> a factory to create a new one when necessary.  Hope that clarifies.
>
> --
> Serge Knystautas
> Lokitech >> software . strategy . design >> http://www.lokitech.com
> p. 301.656.5501
> e. sergek@lokitech.com
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: velocity-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: velocity-dev-help@jakarta.apache.org
>
>


--
_______________________________________
Forio Business Simulations

Will Glass-Husain
phone (415) 440-7500 x89
mobile (415) 235-4293
wglass@forio.com
www.forio.com

Re: SimplePool replacement

Posted by Serge Knystautas <sk...@gmail.com>.
On 3/3/06, Will Glass-Husain <wg...@forio.com> wrote:
> Hi,
>
> I'd argue that SimplePool is a utility class and not part of the public API
> (so it's ok to change this).  Have to draw the line somewhere - otherwise we
> can never change anything.  Anyone object?
>
> I'm not really following the problem with factories -- I'll wait to see
> until I see some code to comment on this.
>
> Llewellyn Falco has a Eclipse code formatting file.  I had a copy but my
> computer is in the shop. Llewellyn are you out there?
>
> Best, WILL

I reran my benchmarks to conclude whether parser pooling is necessary.
 I changed to 0 parsers in the pool (so a new one is created each
template compilation) for these numbers...

4k template file
3.4Ghz HP laptop
Win JDK 1.5
run 1000 times

5,250ms to compile template by reusing parser
6,953ms to compile template creating a parser each time

So given 1,000 iterations, about 1.7ms to create a parser.

WRT my comments about refactoring, the SimplePool is IMO not a pool
but an array.  Right now, RuntimeInstance creates simplepool, iterates
through the # of spots in the pool, constructs a parser and assigns it
to that spot.  Then whenever RuntimeInstance needs a parser, it checks
if one available in simplepool, and if not, it constructs a new one.

I think of a pool as providing object instances and the user does not
know whether it is a reused or new instance.  To do this, a pool needs
a factory to create a new one when necessary.  Hope that clarifies.

--
Serge Knystautas
Lokitech >> software . strategy . design >> http://www.lokitech.com
p. 301.656.5501
e. sergek@lokitech.com

---------------------------------------------------------------------
To unsubscribe, e-mail: velocity-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: velocity-dev-help@jakarta.apache.org


Re: SimplePool replacement

Posted by Serge Knystautas <sk...@gmail.com>.
Ok, all set.  Patch and new classes is in 433 and the commons-pool
impl is in 434.

https://issues.apache.org/jira/browse/VELOCITY-433
https://issues.apache.org/jira/browse/VELOCITY-434
--
Serge Knystautas
Lokitech >> software . strategy . design >> http://www.lokitech.com
p. 301.656.5501
e. sergek@lokitech.com

---------------------------------------------------------------------
To unsubscribe, e-mail: velocity-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: velocity-dev-help@jakarta.apache.org


Re: SimplePool replacement

Posted by Will Glass-Husain <wg...@forio.com>.
good point.  change that to "other issue"

On 3/4/06, Geir Magnusson Jr <ge...@pobox.com> wrote:
>
>
>
> Will Glass-Husain wrote:
> > By the way, it'd be nice to see if the pooling is really necessary as
> > Jonathan suggests.
> > Garbage collection preformance of mainstream JVMs have changed
> significantly
> > since Velocity was first written.  The issue is whether instantiation of
> a
> > javacc parser is still expensive.
>
> Those are two different issues, GC and parser instantiation?
>
> >
> > WILL
> >
> >
> > On 3/3/06, Will Glass-Husain <wg...@forio.com> wrote:
> >>  Hi,
> >>
> >> I'd argue that SimplePool is a utility class and not part of the public
> >> API (so it's ok to change this).  Have to draw the line somewhere -
> >> otherwise we can never change anything.  Anyone object?
> >>
> >> I'm not really following the problem with factories -- I'll wait to see
> >> until I see some code to comment on this.
> >>
> >> Llewellyn Falco has a Eclipse code formatting file.  I had a copy but
> my
> >> computer is in the shop. Llewellyn are you out there?
> >>
> >> Best, WILL
> >>
> >>
> >>> a) I realize that someone might have used SimplePool in their app, so
> >>> renaming the impl and making SimplePool an interface would create an
> >>> upgrade problem.  so creating new classes.
> >>> b) the class is SimplePool and the logic and field is called
> >>> parserPool.  I decided the new classes should use parser pool in the
> >>> names.
> >>> c) pools usually operate with a factory concept, which is hard with
> >>> how the current code is structured (the pool owner populates and
> >>> instantiates extra instances if there's a problem).  I have to
> >>> simulate the factory concept with a callback interface, or something
> >>> like that, which is odd to pass since velocity you usually just
> >>> initialize with the runtimeservices.
> >>> d) code formatting... I don't suppose anyone has the Eclipse code
> >>> formatting rules?  I'm trying to follow conventions as much as
> >>> possible, but if someone can share that, that would be very helpful.
> >>> e) I'll have the commons pool impl available as well, though I doubt
> >>> that would get added into the engine branch.  Where do you think we
> >>> should stick that?
> >>>
> >>> --
> >>> Serge Knystautas
> >>> Lokitech >> software . strategy . design >> http://www.lokitech.com
> >>> p. 301.656.5501
> >>> e. sergek@lokitech.com
> >>>
> >>>
> >>> On 3/2/06, Will Glass-Husain <wg...@forio.com> wrote:
> >>>
> >>>> absolutely. I think for most cases the current implementation is
> >> sufficient,
> >>>> but for your high-rate of create/destroy of runtime instances I can
> see
> >> why
> >>>> this might be a problem.
> >>>>
> >>>> WILL
> >>>>
> >>>> On 3/1/06, Serge Knystautas < sknystautas@gmail.com> wrote:
> >>>>
> >>>>> I did some memory profiling, and for each runtime instance, by
> default
> >>>>> it creates 20 parsers.  According to yourkit, each parser uses just
> >>>>> over 100kb heap space, so each runtime instance is costing me 2mb
> heap
> >>>>> space.
> >>>>>
> >>>>> I can simply reduce the # of parsers, but the problem is that simple
> >>>>> pool is a very, very primitive pool implementation... it holds a
> >>>>> constant number parsers and if you exceed, it simply creates and
> >>>>> disposes a parser (ballpark of 1ms per parser instantiation in my
> >>>>> benchmarks).
> >>>>>
> >>>>> I'd like to convert simplepool to an interface and then make the
> >>>>> current behavior an impl so that you could swap in something like
> >>>>> commons pool.  Is there interest in such a patch?
> >>>>>
> >>>>> --
> >>>>> Serge Knystautas
> >>>>> Lokitech >> software . strategy . design >> http://www.lokitech.com
> >>>>> p. 301.656.5501
> >>>>> e. sergek@lokitech.com
> >>>>>
> >>>>>
> ---------------------------------------------------------------------
> >>>>> To unsubscribe, e-mail: velocity-dev-unsubscribe@jakarta.apache.org
> >>>>> For additional commands, e-mail:
> velocity-dev-help@jakarta.apache.org
> >>>>>
> >>>>>
> >>>>
> >>>> --
> >>>> _______________________________________
> >>>> Forio Business Simulations
> >>>>
> >>>> Will Glass-Husain
> >>>> phone (415) 440-7500 x89
> >>>> mobile (415) 235-4293
> >>>> wglass@forio.com
> >>>> www.forio.com
> >>>>
> >>>>
> >>
> >> ---------------------------------------------------------------------
> >> To unsubscribe, e-mail: velocity-dev-unsubscribe@jakarta.apache.org
> >> For additional commands, e-mail: velocity-dev-help@jakarta.apache.org
> >>
> >>
> >>
> >>
> >>
> >> --
> >>
> >> _______________________________________
> >> Forio Business Simulations
> >>
> >> Will Glass-Husain
> >> phone (415) 440-7500 x89
> >> mobile (415) 235-4293
> >> wglass@forio.com
> >> www.forio.com
> >>
> >
> >
> >
> > --
> > _______________________________________
> > Forio Business Simulations
> >
> > Will Glass-Husain
> > phone (415) 440-7500 x89
> > mobile (415) 235-4293
> > wglass@forio.com
> > www.forio.com
> >
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: velocity-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: velocity-dev-help@jakarta.apache.org
>
>


--
_______________________________________
Forio Business Simulations

Will Glass-Husain
phone (415) 440-7500 x89
mobile (415) 235-4293
wglass@forio.com
www.forio.com

Re: SimplePool replacement

Posted by Geir Magnusson Jr <ge...@pobox.com>.

Will Glass-Husain wrote:
> By the way, it'd be nice to see if the pooling is really necessary as
> Jonathan suggests.
> Garbage collection preformance of mainstream JVMs have changed significantly
> since Velocity was first written.  The issue is whether instantiation of a
> javacc parser is still expensive.

Those are two different issues, GC and parser instantiation?

> 
> WILL
> 
> 
> On 3/3/06, Will Glass-Husain <wg...@forio.com> wrote:
>>  Hi,
>>
>> I'd argue that SimplePool is a utility class and not part of the public
>> API (so it's ok to change this).  Have to draw the line somewhere -
>> otherwise we can never change anything.  Anyone object?
>>
>> I'm not really following the problem with factories -- I'll wait to see
>> until I see some code to comment on this.
>>
>> Llewellyn Falco has a Eclipse code formatting file.  I had a copy but my
>> computer is in the shop. Llewellyn are you out there?
>>
>> Best, WILL
>>
>>
>>> a) I realize that someone might have used SimplePool in their app, so
>>> renaming the impl and making SimplePool an interface would create an
>>> upgrade problem.  so creating new classes.
>>> b) the class is SimplePool and the logic and field is called
>>> parserPool.  I decided the new classes should use parser pool in the
>>> names.
>>> c) pools usually operate with a factory concept, which is hard with
>>> how the current code is structured (the pool owner populates and
>>> instantiates extra instances if there's a problem).  I have to
>>> simulate the factory concept with a callback interface, or something
>>> like that, which is odd to pass since velocity you usually just
>>> initialize with the runtimeservices.
>>> d) code formatting... I don't suppose anyone has the Eclipse code
>>> formatting rules?  I'm trying to follow conventions as much as
>>> possible, but if someone can share that, that would be very helpful.
>>> e) I'll have the commons pool impl available as well, though I doubt
>>> that would get added into the engine branch.  Where do you think we
>>> should stick that?
>>>
>>> --
>>> Serge Knystautas
>>> Lokitech >> software . strategy . design >> http://www.lokitech.com
>>> p. 301.656.5501
>>> e. sergek@lokitech.com
>>>
>>>
>>> On 3/2/06, Will Glass-Husain <wg...@forio.com> wrote:
>>>
>>>> absolutely. I think for most cases the current implementation is
>> sufficient,
>>>> but for your high-rate of create/destroy of runtime instances I can see
>> why
>>>> this might be a problem.
>>>>
>>>> WILL
>>>>
>>>> On 3/1/06, Serge Knystautas < sknystautas@gmail.com> wrote:
>>>>
>>>>> I did some memory profiling, and for each runtime instance, by default
>>>>> it creates 20 parsers.  According to yourkit, each parser uses just
>>>>> over 100kb heap space, so each runtime instance is costing me 2mb heap
>>>>> space.
>>>>>
>>>>> I can simply reduce the # of parsers, but the problem is that simple
>>>>> pool is a very, very primitive pool implementation... it holds a
>>>>> constant number parsers and if you exceed, it simply creates and
>>>>> disposes a parser (ballpark of 1ms per parser instantiation in my
>>>>> benchmarks).
>>>>>
>>>>> I'd like to convert simplepool to an interface and then make the
>>>>> current behavior an impl so that you could swap in something like
>>>>> commons pool.  Is there interest in such a patch?
>>>>>
>>>>> --
>>>>> Serge Knystautas
>>>>> Lokitech >> software . strategy . design >> http://www.lokitech.com
>>>>> p. 301.656.5501
>>>>> e. sergek@lokitech.com
>>>>>
>>>>> ---------------------------------------------------------------------
>>>>> To unsubscribe, e-mail: velocity-dev-unsubscribe@jakarta.apache.org
>>>>> For additional commands, e-mail: velocity-dev-help@jakarta.apache.org
>>>>>
>>>>>
>>>>
>>>> --
>>>> _______________________________________
>>>> Forio Business Simulations
>>>>
>>>> Will Glass-Husain
>>>> phone (415) 440-7500 x89
>>>> mobile (415) 235-4293
>>>> wglass@forio.com
>>>> www.forio.com
>>>>
>>>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: velocity-dev-unsubscribe@jakarta.apache.org
>> For additional commands, e-mail: velocity-dev-help@jakarta.apache.org
>>
>>
>>
>>
>>
>> --
>>
>> _______________________________________
>> Forio Business Simulations
>>
>> Will Glass-Husain
>> phone (415) 440-7500 x89
>> mobile (415) 235-4293
>> wglass@forio.com
>> www.forio.com
>>
> 
> 
> 
> --
> _______________________________________
> Forio Business Simulations
> 
> Will Glass-Husain
> phone (415) 440-7500 x89
> mobile (415) 235-4293
> wglass@forio.com
> www.forio.com
> 

---------------------------------------------------------------------
To unsubscribe, e-mail: velocity-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: velocity-dev-help@jakarta.apache.org


Re: SimplePool replacement

Posted by Will Glass-Husain <wg...@forio.com>.
By the way, it'd be nice to see if the pooling is really necessary as
Jonathan suggests.
Garbage collection preformance of mainstream JVMs have changed significantly
since Velocity was first written.  The issue is whether instantiation of a
javacc parser is still expensive.

WILL


On 3/3/06, Will Glass-Husain <wg...@forio.com> wrote:
>
>  Hi,
>
> I'd argue that SimplePool is a utility class and not part of the public
> API (so it's ok to change this).  Have to draw the line somewhere -
> otherwise we can never change anything.  Anyone object?
>
> I'm not really following the problem with factories -- I'll wait to see
> until I see some code to comment on this.
>
> Llewellyn Falco has a Eclipse code formatting file.  I had a copy but my
> computer is in the shop. Llewellyn are you out there?
>
> Best, WILL
>
>
> >
> > a) I realize that someone might have used SimplePool in their app, so
> > renaming the impl and making SimplePool an interface would create an
> > upgrade problem.  so creating new classes.
> > b) the class is SimplePool and the logic and field is called
> > parserPool.  I decided the new classes should use parser pool in the
> > names.
> > c) pools usually operate with a factory concept, which is hard with
> > how the current code is structured (the pool owner populates and
> > instantiates extra instances if there's a problem).  I have to
> > simulate the factory concept with a callback interface, or something
> > like that, which is odd to pass since velocity you usually just
> > initialize with the runtimeservices.
> > d) code formatting... I don't suppose anyone has the Eclipse code
> > formatting rules?  I'm trying to follow conventions as much as
> > possible, but if someone can share that, that would be very helpful.
> > e) I'll have the commons pool impl available as well, though I doubt
> > that would get added into the engine branch.  Where do you think we
> > should stick that?
> >
> > --
> > Serge Knystautas
> > Lokitech >> software . strategy . design >> http://www.lokitech.com
> > p. 301.656.5501
> > e. sergek@lokitech.com
> >
> >
> > On 3/2/06, Will Glass-Husain <wg...@forio.com> wrote:
> >
> >>absolutely. I think for most cases the current implementation is
> sufficient,
> >>but for your high-rate of create/destroy of runtime instances I can see
> why
> >>this might be a problem.
> >>
> >>WILL
> >>
> >>On 3/1/06, Serge Knystautas < sknystautas@gmail.com> wrote:
> >>
> >>>I did some memory profiling, and for each runtime instance, by default
> >>>it creates 20 parsers.  According to yourkit, each parser uses just
> >>>over 100kb heap space, so each runtime instance is costing me 2mb heap
> >>>space.
> >>>
> >>>I can simply reduce the # of parsers, but the problem is that simple
> >>>pool is a very, very primitive pool implementation... it holds a
> >>>constant number parsers and if you exceed, it simply creates and
> >>>disposes a parser (ballpark of 1ms per parser instantiation in my
> >>>benchmarks).
> >>>
> >>>I'd like to convert simplepool to an interface and then make the
> >>>current behavior an impl so that you could swap in something like
> >>>commons pool.  Is there interest in such a patch?
> >>>
> >>>--
> >>>Serge Knystautas
> >>>Lokitech >> software . strategy . design >> http://www.lokitech.com
> >>>p. 301.656.5501
> >>>e. sergek@lokitech.com
> >>>
> >>>---------------------------------------------------------------------
> >>>To unsubscribe, e-mail: velocity-dev-unsubscribe@jakarta.apache.org
> >>>For additional commands, e-mail: velocity-dev-help@jakarta.apache.org
> >>>
> >>>
> >>
> >>
> >>--
> >>_______________________________________
> >>Forio Business Simulations
> >>
> >>Will Glass-Husain
> >>phone (415) 440-7500 x89
> >>mobile (415) 235-4293
> >>wglass@forio.com
> >>www.forio.com
> >>
> >>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: velocity-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: velocity-dev-help@jakarta.apache.org
>
>
>
>
>
> --
>
> _______________________________________
> Forio Business Simulations
>
> Will Glass-Husain
> phone (415) 440-7500 x89
> mobile (415) 235-4293
> wglass@forio.com
> www.forio.com
>



--
_______________________________________
Forio Business Simulations

Will Glass-Husain
phone (415) 440-7500 x89
mobile (415) 235-4293
wglass@forio.com
www.forio.com

Re: SimplePool replacement

Posted by Will Glass-Husain <wg...@forio.com>.
Hi,

I'd argue that SimplePool is a utility class and not part of the public API
(so it's ok to change this).  Have to draw the line somewhere - otherwise we
can never change anything.  Anyone object?

I'm not really following the problem with factories -- I'll wait to see
until I see some code to comment on this.

Llewellyn Falco has a Eclipse code formatting file.  I had a copy but my
computer is in the shop. Llewellyn are you out there?

Best, WILL


>
> a) I realize that someone might have used SimplePool in their app, so
> renaming the impl and making SimplePool an interface would create an
> upgrade problem.  so creating new classes.
> b) the class is SimplePool and the logic and field is called
> parserPool.  I decided the new classes should use parser pool in the
> names.
> c) pools usually operate with a factory concept, which is hard with
> how the current code is structured (the pool owner populates and
> instantiates extra instances if there's a problem).  I have to
> simulate the factory concept with a callback interface, or something
> like that, which is odd to pass since velocity you usually just
> initialize with the runtimeservices.
> d) code formatting... I don't suppose anyone has the Eclipse code
> formatting rules?  I'm trying to follow conventions as much as
> possible, but if someone can share that, that would be very helpful.
> e) I'll have the commons pool impl available as well, though I doubt
> that would get added into the engine branch.  Where do you think we
> should stick that?
>
> --
> Serge Knystautas
> Lokitech >> software . strategy . design >> http://www.lokitech.com
> p. 301.656.5501
> e. sergek@lokitech.com
>
>
> On 3/2/06, Will Glass-Husain <wg...@forio.com> wrote:
>
>>absolutely. I think for most cases the current implementation is
sufficient,
>>but for your high-rate of create/destroy of runtime instances I can see
why
>>this might be a problem.
>>
>>WILL
>>
>>On 3/1/06, Serge Knystautas <sk...@gmail.com> wrote:
>>
>>>I did some memory profiling, and for each runtime instance, by default
>>>it creates 20 parsers.  According to yourkit, each parser uses just
>>>over 100kb heap space, so each runtime instance is costing me 2mb heap
>>>space.
>>>
>>>I can simply reduce the # of parsers, but the problem is that simple
>>>pool is a very, very primitive pool implementation... it holds a
>>>constant number parsers and if you exceed, it simply creates and
>>>disposes a parser (ballpark of 1ms per parser instantiation in my
>>>benchmarks).
>>>
>>>I'd like to convert simplepool to an interface and then make the
>>>current behavior an impl so that you could swap in something like
>>>commons pool.  Is there interest in such a patch?
>>>
>>>--
>>>Serge Knystautas
>>>Lokitech >> software . strategy . design >> http://www.lokitech.com
>>>p. 301.656.5501
>>>e. sergek@lokitech.com
>>>
>>>---------------------------------------------------------------------
>>>To unsubscribe, e-mail: velocity-dev-unsubscribe@jakarta.apache.org
>>>For additional commands, e-mail: velocity-dev-help@jakarta.apache.org
>>>
>>>
>>
>>
>>--
>>_______________________________________
>>Forio Business Simulations
>>
>>Will Glass-Husain
>>phone (415) 440-7500 x89
>>mobile (415) 235-4293
>>wglass@forio.com
>>www.forio.com
>>
>>


---------------------------------------------------------------------
To unsubscribe, e-mail: velocity-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: velocity-dev-help@jakarta.apache.org





--
_______________________________________
Forio Business Simulations

Will Glass-Husain
phone (415) 440-7500 x89
mobile (415) 235-4293
wglass@forio.com
www.forio.com

Re: SimplePool replacement

Posted by Jonathan Revusky <re...@wanadoo.es>.
Serge Knystautas wrote:
> This is turning into a bigger refactor than expected, so wanted to run
> ideas past everyone...

Out of idle curiosity, why does Velocity maintain a pool of 20 parsers? 
Is instantiating a new Parser object via 'new' actually that costly? 
FreeMarker just instantiates them via something like:

FMParser parser = new FMParser(theReader, <and some other params>);

There is no pooling. If the pooling doesn't actually buy anything 
significant, then maybe what you really want to do is just get rid of it 
completely.

(Thought that comment could be helpful....)

Jonathan Revusky
--
lead developer, FreeMarker project, http://freemarker.org/


> 
> a) I realize that someone might have used SimplePool in their app, so
> renaming the impl and making SimplePool an interface would create an
> upgrade problem.  so creating new classes.
> b) the class is SimplePool and the logic and field is called
> parserPool.  I decided the new classes should use parser pool in the
> names.
> c) pools usually operate with a factory concept, which is hard with
> how the current code is structured (the pool owner populates and
> instantiates extra instances if there's a problem).  I have to
> simulate the factory concept with a callback interface, or something
> like that, which is odd to pass since velocity you usually just
> initialize with the runtimeservices.
> d) code formatting... I don't suppose anyone has the Eclipse code
> formatting rules?  I'm trying to follow conventions as much as
> possible, but if someone can share that, that would be very helpful.
> e) I'll have the commons pool impl available as well, though I doubt
> that would get added into the engine branch.  Where do you think we
> should stick that?
> 
> --
> Serge Knystautas
> Lokitech >> software . strategy . design >> http://www.lokitech.com
> p. 301.656.5501
> e. sergek@lokitech.com
> 
> 
> On 3/2/06, Will Glass-Husain <wg...@forio.com> wrote:
> 
>>absolutely. I think for most cases the current implementation is sufficient,
>>but for your high-rate of create/destroy of runtime instances I can see why
>>this might be a problem.
>>
>>WILL
>>
>>On 3/1/06, Serge Knystautas <sk...@gmail.com> wrote:
>>
>>>I did some memory profiling, and for each runtime instance, by default
>>>it creates 20 parsers.  According to yourkit, each parser uses just
>>>over 100kb heap space, so each runtime instance is costing me 2mb heap
>>>space.
>>>
>>>I can simply reduce the # of parsers, but the problem is that simple
>>>pool is a very, very primitive pool implementation... it holds a
>>>constant number parsers and if you exceed, it simply creates and
>>>disposes a parser (ballpark of 1ms per parser instantiation in my
>>>benchmarks).
>>>
>>>I'd like to convert simplepool to an interface and then make the
>>>current behavior an impl so that you could swap in something like
>>>commons pool.  Is there interest in such a patch?
>>>
>>>--
>>>Serge Knystautas
>>>Lokitech >> software . strategy . design >> http://www.lokitech.com
>>>p. 301.656.5501
>>>e. sergek@lokitech.com
>>>
>>>---------------------------------------------------------------------
>>>To unsubscribe, e-mail: velocity-dev-unsubscribe@jakarta.apache.org
>>>For additional commands, e-mail: velocity-dev-help@jakarta.apache.org
>>>
>>>
>>
>>
>>--
>>_______________________________________
>>Forio Business Simulations
>>
>>Will Glass-Husain
>>phone (415) 440-7500 x89
>>mobile (415) 235-4293
>>wglass@forio.com
>>www.forio.com
>>
>>


---------------------------------------------------------------------
To unsubscribe, e-mail: velocity-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: velocity-dev-help@jakarta.apache.org


Re: SimplePool replacement

Posted by Serge Knystautas <sk...@gmail.com>.
This is turning into a bigger refactor than expected, so wanted to run
ideas past everyone...

a) I realize that someone might have used SimplePool in their app, so
renaming the impl and making SimplePool an interface would create an
upgrade problem.  so creating new classes.
b) the class is SimplePool and the logic and field is called
parserPool.  I decided the new classes should use parser pool in the
names.
c) pools usually operate with a factory concept, which is hard with
how the current code is structured (the pool owner populates and
instantiates extra instances if there's a problem).  I have to
simulate the factory concept with a callback interface, or something
like that, which is odd to pass since velocity you usually just
initialize with the runtimeservices.
d) code formatting... I don't suppose anyone has the Eclipse code
formatting rules?  I'm trying to follow conventions as much as
possible, but if someone can share that, that would be very helpful.
e) I'll have the commons pool impl available as well, though I doubt
that would get added into the engine branch.  Where do you think we
should stick that?

--
Serge Knystautas
Lokitech >> software . strategy . design >> http://www.lokitech.com
p. 301.656.5501
e. sergek@lokitech.com


On 3/2/06, Will Glass-Husain <wg...@forio.com> wrote:
> absolutely. I think for most cases the current implementation is sufficient,
> but for your high-rate of create/destroy of runtime instances I can see why
> this might be a problem.
>
> WILL
>
> On 3/1/06, Serge Knystautas <sk...@gmail.com> wrote:
> >
> > I did some memory profiling, and for each runtime instance, by default
> > it creates 20 parsers.  According to yourkit, each parser uses just
> > over 100kb heap space, so each runtime instance is costing me 2mb heap
> > space.
> >
> > I can simply reduce the # of parsers, but the problem is that simple
> > pool is a very, very primitive pool implementation... it holds a
> > constant number parsers and if you exceed, it simply creates and
> > disposes a parser (ballpark of 1ms per parser instantiation in my
> > benchmarks).
> >
> > I'd like to convert simplepool to an interface and then make the
> > current behavior an impl so that you could swap in something like
> > commons pool.  Is there interest in such a patch?
> >
> > --
> > Serge Knystautas
> > Lokitech >> software . strategy . design >> http://www.lokitech.com
> > p. 301.656.5501
> > e. sergek@lokitech.com
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: velocity-dev-unsubscribe@jakarta.apache.org
> > For additional commands, e-mail: velocity-dev-help@jakarta.apache.org
> >
> >
>
>
> --
> _______________________________________
> Forio Business Simulations
>
> Will Glass-Husain
> phone (415) 440-7500 x89
> mobile (415) 235-4293
> wglass@forio.com
> www.forio.com
>
>

---------------------------------------------------------------------
To unsubscribe, e-mail: velocity-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: velocity-dev-help@jakarta.apache.org


Re: SimplePool replacement

Posted by Will Glass-Husain <wg...@forio.com>.
absolutely. I think for most cases the current implementation is sufficient,
but for your high-rate of create/destroy of runtime instances I can see why
this might be a problem.

WILL

On 3/1/06, Serge Knystautas <sk...@gmail.com> wrote:
>
> I did some memory profiling, and for each runtime instance, by default
> it creates 20 parsers.  According to yourkit, each parser uses just
> over 100kb heap space, so each runtime instance is costing me 2mb heap
> space.
>
> I can simply reduce the # of parsers, but the problem is that simple
> pool is a very, very primitive pool implementation... it holds a
> constant number parsers and if you exceed, it simply creates and
> disposes a parser (ballpark of 1ms per parser instantiation in my
> benchmarks).
>
> I'd like to convert simplepool to an interface and then make the
> current behavior an impl so that you could swap in something like
> commons pool.  Is there interest in such a patch?
>
> --
> Serge Knystautas
> Lokitech >> software . strategy . design >> http://www.lokitech.com
> p. 301.656.5501
> e. sergek@lokitech.com
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: velocity-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: velocity-dev-help@jakarta.apache.org
>
>


--
_______________________________________
Forio Business Simulations

Will Glass-Husain
phone (415) 440-7500 x89
mobile (415) 235-4293
wglass@forio.com
www.forio.com

Re: SimplePool replacement

Posted by "Henning P. Schmiedehausen" <hp...@intermeta.de>.
"Serge Knystautas" <sk...@gmail.com> writes:

>I'd like to convert simplepool to an interface and then make the
>current behavior an impl so that you could swap in something like
>commons pool.  Is there interest in such a patch?

Definitely. We are always interested in patches... :-)

	Best regards
		Henning

-- 
Dipl.-Inf. (Univ.) Henning P. Schmiedehausen          INTERMETA GmbH
hps@intermeta.de        +49 9131 50 654 0   http://www.intermeta.de/

RedHat Certified Engineer -- Jakarta Turbine Development  -- hero for hire
   Linux, Java, perl, Solaris -- Consulting, Training, Development

Social behaviour: Bavarians can be extremely egalitarian and folksy.
                                    -- http://en.wikipedia.org/wiki/Bavaria
Most Franconians do not like to be called Bavarians.
                                    -- http://en.wikipedia.org/wiki/Franconia

---------------------------------------------------------------------
To unsubscribe, e-mail: velocity-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: velocity-dev-help@jakarta.apache.org


Re: SimplePool replacement

Posted by Geir Magnusson Jr <ge...@pobox.com>.
Yes.

Serge Knystautas wrote:
> I did some memory profiling, and for each runtime instance, by default
> it creates 20 parsers.  According to yourkit, each parser uses just
> over 100kb heap space, so each runtime instance is costing me 2mb heap
> space.
> 
> I can simply reduce the # of parsers, but the problem is that simple
> pool is a very, very primitive pool implementation... it holds a
> constant number parsers and if you exceed, it simply creates and
> disposes a parser (ballpark of 1ms per parser instantiation in my
> benchmarks).
> 
> I'd like to convert simplepool to an interface and then make the
> current behavior an impl so that you could swap in something like
> commons pool.  Is there interest in such a patch?
> 
> --
> Serge Knystautas
> Lokitech >> software . strategy . design >> http://www.lokitech.com
> p. 301.656.5501
> e. sergek@lokitech.com
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: velocity-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: velocity-dev-help@jakarta.apache.org
> 
> 

---------------------------------------------------------------------
To unsubscribe, e-mail: velocity-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: velocity-dev-help@jakarta.apache.org