Re: [Proposal] Preparatory refactoring for Resource handling refactoring

[Konstantin Kolinko <kn...@gmail.com>]

(regarding DefaultServlet)
>> If it were using the servlet API only, the code were more reusable,
>> both inside and outside of Tomcat. There must be specific issue why
>> Servlet API is not used there. Is the API lacking?
>
> In lot of ways. For example, where is the API to create or delete
> resources? That is deliberately left as a container implementation detail.
>

OK.  One way is to use getRealPath() + manipulate files on the file
system directly. I agree that using the API (be it DirContext or
something else) is a bit more nice.

>
> The jndi layer has caused performance problems as well. The special
> handling for JARs is a direct result of that. The new approach will
> simplify a lot of that and hopefully improve performance as well. Like
> we have with the current approach, if there are specific areas causing
> problems, we can take a look. I hope and expect that far fewer of these
> 'tweaks' would be required with the new implementation.
>
>>>> One good thing with "jndi:" URLs returned via Servlet API is that they
>>>> are backed by an instance of ProxyDirContext class and it has a cache
>>>> (*).  If we change implementation and return "true" URLs, they will
>>>> bypass the cache.  I suspect that using a "jar:" URL directly (in case
>>>> of an unpacked WAR file) may have poor performance.
>>>
>>> The new Resources implementation will include caching where necessary.
>>> At the moment there is none. I intend to add it as required. I agree
>>> JARs/WARs are likely to need it to have performance that is comparable
>>> with expanded WARs.
>>>

Regarding this caching:
1. Currently calling getInputStream() on a jndi URL may return
resource directly from memory (cached by ProxyDirContext).

If a file URL is used that would involve direct reading from hard
drive.  If a jar URL is used - I wonder whether opening of the JAR
file by the system had to be performed regardless of independent
access to the same file.

One may argue though that there is
ServletContext.getResourceAsStream() method available.

2. If I query some attributes of the resource via
getResource().openConnection().getHeaderField(),  for jndi URLs they
are returned from a cache. I wonder what performance will be there
with file and jar URLs.

>
> I can put a patch of the changes so far on people.a.o if you are
> interested (note: it won't compile yet).
>

I would like to look and review.  I think that at the current point
this refactoring should not be done directly on trunk without some
review and consensus first.

Maybe create a branch for this work (create a "tomcat/branches/"
directory, or use existing "tomcat/sandbox")?


Best regards,
Konstantin Kolinko

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

Re: [Proposal] Preparatory refactoring for Resource handling refactoring

[Mark Thomas <ma...@apache.org>]

On 17/08/2012 16:50, Konstantin Kolinko wrote:
> (regarding DefaultServlet)
>>> If it were using the servlet API only, the code were more reusable,
>>> both inside and outside of Tomcat. There must be specific issue why
>>> Servlet API is not used there. Is the API lacking?
>>
>> In lot of ways. For example, where is the API to create or delete
>> resources? That is deliberately left as a container implementation detail.
>>
> 
> OK.  One way is to use getRealPath() + manipulate files on the file
> system directly. I agree that using the API (be it DirContext or
> something else) is a bit more nice.

getRealPath() won't work for read/write storage that isn't backed by a
file system (e.g. database backed)

It also gets rather expensive when you have to deal with static
resources from multiple sources. You end up needing a several calls to
work out a) if the resource exists, b) if it can be put / deleted
depending on whether the resource is from a resource JAR, collides with
one from a resource JAR etc. Not impossible, but you need quite a few
calls to get it right. This gets more complicated when you add overlays
to the mix.

>> The jndi layer has caused performance problems as well. The special
>> handling for JARs is a direct result of that. The new approach will
>> simplify a lot of that and hopefully improve performance as well. Like
>> we have with the current approach, if there are specific areas causing
>> problems, we can take a look. I hope and expect that far fewer of these
>> 'tweaks' would be required with the new implementation.
>>
>>>>> One good thing with "jndi:" URLs returned via Servlet API is that they
>>>>> are backed by an instance of ProxyDirContext class and it has a cache
>>>>> (*).  If we change implementation and return "true" URLs, they will
>>>>> bypass the cache.  I suspect that using a "jar:" URL directly (in case
>>>>> of an unpacked WAR file) may have poor performance.
>>>>
>>>> The new Resources implementation will include caching where necessary.
>>>> At the moment there is none. I intend to add it as required. I agree
>>>> JARs/WARs are likely to need it to have performance that is comparable
>>>> with expanded WARs.
>>>>
> 
> Regarding this caching:
> 1. Currently calling getInputStream() on a jndi URL may return
> resource directly from memory (cached by ProxyDirContext).
> 
> If a file URL is used that would involve direct reading from hard
> drive.  If a jar URL is used - I wonder whether opening of the JAR
> file by the system had to be performed regardless of independent
> access to the same file.
> 
> One may argue though that there is
> ServletContext.getResourceAsStream() method available.
> 
> 2. If I query some attributes of the resource via
> getResource().openConnection().getHeaderField(),  for jndi URLs they
> are returned from a cache. I wonder what performance will be there
> with file and jar URLs.

I remain to be convinced how much caching is required. My preference is
to add a very simple (possibly even NO-OP) caching layer that we can
extend as necessary to address actual performance issues rather than
guessing where they might occur.

>> I can put a patch of the changes so far on people.a.o if you are
>> interested (note: it won't compile yet).
>>
> 
> I would like to look and review.  I think that at the current point
> this refactoring should not be done directly on trunk without some
> review and consensus first.

I've almost got something working. I'll see if I can finish it off
first. Either way I'll aim to post something this coming weekend.

> Maybe create a branch for this work (create a "tomcat/branches/"
> directory, or use existing "tomcat/sandbox")?

I'm happy with that if required. I suggest we hold of on the how until
folks have had a chance to look at the patch first.

Mark


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