You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@velocity.apache.org by Paulo Gaspar <pa...@krankikom.de> on 2001/02/19 23:15:39 UTC

Reource loading issues

Here I go again...


The is a waste of performance and a bug on resource loading
in the ResourceManager class.

* The Bug *

In ResourceManager.getResource(), loading a resource with
  resource.process();
always happens before reading its "LastModified" timestamp
with
  resource.setLastModified(resourceLoader.getLastModified(resource));

The bug is that this can cause a change to become undetected
if one has this sequence of events:
  1 - resource is loaded;
  2 - resource is changed at its source (file or other);
  3 - timestamp is red.

In 3, this would be the timestamp of the new version instead
of the old. Further timestamp checks would not detect the 
change because the timestamp red does NOT belong to the 
resource red.


* The performance waste *

Besides consistency advantages, it is usually faster to read 
the resource's text and its timestamp in a single operation. 
This is especially obvious with database resource loaders
like David's (2 database operations instead of one in order
to load a template).

However, the current ResourceManager/ResourceLoader 
mechanism does not allow that.


Have fun,
Paulo Gaspar


Re: Reource loading issues

Posted by "Geir Magnusson Jr." <ge...@optonline.net>.
Nothing much more to add to Jason's reply as I am sick-like-dog and
don't want to read code :)

But I assume this takes into account small fix I did friday-ish?

geir


Paulo Gaspar wrote:
> 
> Here I go again...
> 
> The is a waste of performance and a bug on resource loading
> in the ResourceManager class.
> 
> * The Bug *
> 
> In ResourceManager.getResource(), loading a resource with
>   resource.process();
> always happens before reading its "LastModified" timestamp
> with
>   resource.setLastModified(resourceLoader.getLastModified(resource));
> 
> The bug is that this can cause a change to become undetected
> if one has this sequence of events:
>   1 - resource is loaded;
>   2 - resource is changed at its source (file or other);
>   3 - timestamp is red.
> 
> In 3, this would be the timestamp of the new version instead
> of the old. Further timestamp checks would not detect the
> change because the timestamp red does NOT belong to the
> resource red.
> 
> * The performance waste *
> 
> Besides consistency advantages, it is usually faster to read
> the resource's text and its timestamp in a single operation.
> This is especially obvious with database resource loaders
> like David's (2 database operations instead of one in order
> to load a template).
> 
> However, the current ResourceManager/ResourceLoader
> mechanism does not allow that.
> 
> Have fun,
> Paulo Gaspar

-- 
Geir Magnusson Jr.                               geirm@optonline.com
Velocity : it's not just a good idea. It should be the law.
http://jakarta.apache.org/velocity

Re: Reource loading issues

Posted by Daniel Rall <dl...@collab.net>.
"Paulo Gaspar" <pa...@krankikom.de> writes:
> Anyway, I will fix the bug, but I am afraid I don't know how 
> to make patches.
> =:o/
> 
> I am taking a look at cygwin to see if I can figure out how to 
> do it.

I think that you can do it via WinCVS as well (mind you, I don't use
WinCVS).
-- 

Daniel Rall <dl...@collab.net>

RE: Reource loading issues

Posted by Paulo Gaspar <pa...@krankikom.de>.
Thanks Geir,

Paulo

> -----Original Message-----
> From: gmj@mta2.srv.hcvlny.cv.net [mailto:gmj@mta2.srv.hcvlny.cv.net]On
> Behalf Of Geir Magnusson Jr.
> 
> 
> Paulo Gaspar wrote:
> > 
> > Messing up with that "performance waste" is a bit delicate. It
> > involves design decisions.
> 
> Free free to raise any issues here...
> 
> ....


Re: Reource loading issues

Posted by "Geir Magnusson Jr." <ge...@optonline.net>.
Paulo Gaspar wrote:
> 
> Messing up with that "performance waste" is a bit delicate. It
> involves design decisions.

Free free to raise any issues here...


> Anyway, I will fix the bug, but I am afraid I don't know how
> to make patches.
> =:o/
> 
> I am taking a look at cygwin to see if I can figure out how to
> do it.
> 
> Have fun,
> Paulo
> 
> > -----Original Message-----
> > From: Jason van Zyl [mailto:jvanzyl@periapt.com]
> > Sent: Monday, February 19, 2001 23:23
> >
> >
> >
> > Thanks, noted. I won't get to this for a while so if
> > you like:
> >
> > 1. fix it
> > 2. test it
> > 3. send the patches please
> >
> > :-)
> >
> > jvz.
> >
> > On Mon, 19 Feb 2001, Paulo Gaspar wrote:
> >
> > > .........

-- 
Geir Magnusson Jr.                               geirm@optonline.com
Velocity : it's not just a good idea. It should be the law.
http://jakarta.apache.org/velocity

RE: Reource loading issues

Posted by Paulo Gaspar <pa...@krankikom.de>.
The "This keeps it atomic-ish" part sounds very good. However, I still 
do not understand how Resources are handled well enough to understand
how I have to remember the date.

At the moment I am even wondering why we have both
  ResourceLoader.isSourceModified()
and
  ResourceLoader.getLastModified()

Probably they make sense _after_ I understand it all.
=:o\


I am busy with osme other stuff yet, but this will become my main 
project in some hours.
=:o)


Thanks and get better Geir,
Paulo


> -----Original Message-----
> From: gmj@mta6.srv.hcvlny.cv.net [mailto:gmj@mta6.srv.hcvlny.cv.net]On
> Behalf Of Geir Magnusson Jr.
> 
> 
> Paulo Gaspar wrote:
> > 
> > Messing up with that "performance waste" is a bit delicate. It
> > involves design decisions.
> 
> Couldn't you take one of the following approachs  : 
> 
> Since a resource is managed entirely by it's loader, why not let the
> loader simply read the timestamp of the resource during
> getResourceStream() and remember it. This keeps it atomic-ish. Then :
> 
> 1) ResourceManager would get that timestamp and set it in the resource
> in getResource() like it does now, via a different call, such as
> resourceLoader.getResourceModified( resource )
> 
> OR
> 
> 2) Let the resource itself get the timestamp when it calls
> getResourceStream() from it's loader.  This keeps the operation atomic
> as far as the ResourceManager is concerned.
> 
> Either way, having the Loader maintain it seems the best thing, and
> neither of the above requires any serious design issue, right?
> 
> geir
> 


Re: Reource loading issues

Posted by "Geir Magnusson Jr." <ge...@optonline.net>.
Paulo Gaspar wrote:
> 
> Messing up with that "performance waste" is a bit delicate. It
> involves design decisions.

Couldn't you take one of the following approachs  : 

Since a resource is managed entirely by it's loader, why not let the
loader simply read the timestamp of the resource during
getResourceStream() and remember it. This keeps it atomic-ish. Then :

1) ResourceManager would get that timestamp and set it in the resource
in getResource() like it does now, via a different call, such as
resourceLoader.getResourceModified( resource )

OR

2) Let the resource itself get the timestamp when it calls
getResourceStream() from it's loader.  This keeps the operation atomic
as far as the ResourceManager is concerned.

Either way, having the Loader maintain it seems the best thing, and
neither of the above requires any serious design issue, right?

geir


 
> 
> Anyway, I will fix the bug, but I am afraid I don't know how
> to make patches.
> =:o/
> 
> I am taking a look at cygwin to see if I can figure out how to
> do it.
> 
> Have fun,
> Paulo
> 
> > -----Original Message-----
> > From: Jason van Zyl [mailto:jvanzyl@periapt.com]
> > Sent: Monday, February 19, 2001 23:23
> >
> >
> >
> > Thanks, noted. I won't get to this for a while so if
> > you like:
> >
> > 1. fix it
> > 2. test it
> > 3. send the patches please
> >
> > :-)
> >
> > jvz.
> >
> > On Mon, 19 Feb 2001, Paulo Gaspar wrote:
> >
> > > .........

-- 
Geir Magnusson Jr.                               geirm@optonline.com
Velocity : it's not just a good idea. It should be the law.
http://jakarta.apache.org/velocity

RE: Reource loading issues

Posted by Paulo Gaspar <pa...@krankikom.de>.
Messing up with that "performance waste" is a bit delicate. It 
involves design decisions.

Anyway, I will fix the bug, but I am afraid I don't know how 
to make patches.
=:o/

I am taking a look at cygwin to see if I can figure out how to 
do it.


Have fun,
Paulo


> -----Original Message-----
> From: Jason van Zyl [mailto:jvanzyl@periapt.com]
> Sent: Monday, February 19, 2001 23:23
> 
> 
> 
> Thanks, noted. I won't get to this for a while so if
> you like:
> 
> 1. fix it
> 2. test it
> 3. send the patches please
> 
> :-)
> 
> jvz.
> 
> On Mon, 19 Feb 2001, Paulo Gaspar wrote:
> 
> > .........


Re: Reource loading issues

Posted by Jason van Zyl <jv...@periapt.com>.
Thanks, noted. I won't get to this for a while so if
you like:

1. fix it
2. test it
3. send the patches please

:-)

jvz.

On Mon, 19 Feb 2001, Paulo Gaspar wrote:

> Here I go again...
> 
> 
> The is a waste of performance and a bug on resource loading
> in the ResourceManager class.
> 
> * The Bug *
> 
> In ResourceManager.getResource(), loading a resource with
>   resource.process();
> always happens before reading its "LastModified" timestamp
> with
>   resource.setLastModified(resourceLoader.getLastModified(resource));
> 
> The bug is that this can cause a change to become undetected
> if one has this sequence of events:
>   1 - resource is loaded;
>   2 - resource is changed at its source (file or other);
>   3 - timestamp is red.
> 
> In 3, this would be the timestamp of the new version instead
> of the old. Further timestamp checks would not detect the 
> change because the timestamp red does NOT belong to the 
> resource red.
> 
> 
> * The performance waste *
> 
> Besides consistency advantages, it is usually faster to read 
> the resource's text and its timestamp in a single operation. 
> This is especially obvious with database resource loaders
> like David's (2 database operations instead of one in order
> to load a template).
> 
> However, the current ResourceManager/ResourceLoader 
> mechanism does not allow that.
> 
> 
> Have fun,
> Paulo Gaspar
> 
>