Return-Path: Delivered-To: apmail-click-dev-archive@www.apache.org Received: (qmail 90115 invoked from network); 15 Jun 2010 13:05:35 -0000 Received: from unknown (HELO mail.apache.org) (140.211.11.3) by 140.211.11.9 with SMTP; 15 Jun 2010 13:05:35 -0000 Received: (qmail 23963 invoked by uid 500); 15 Jun 2010 13:05:35 -0000 Delivered-To: apmail-click-dev-archive@click.apache.org Received: (qmail 23891 invoked by uid 500); 15 Jun 2010 13:05:34 -0000 Mailing-List: contact dev-help@click.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@click.apache.org Delivered-To: mailing list dev@click.apache.org Received: (qmail 23880 invoked by uid 99); 15 Jun 2010 13:05:33 -0000 Received: from nike.apache.org (HELO nike.apache.org) (192.87.106.230) by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 15 Jun 2010 13:05:33 +0000 X-ASF-Spam-Status: No, hits=2.2 required=10.0 tests=FREEMAIL_FROM,HTML_MESSAGE,SPF_PASS,T_TO_NO_BRKTS_FREEMAIL X-Spam-Check-By: apache.org Received-SPF: pass (nike.apache.org: domain of development.jsh@gmail.com designates 74.125.82.170 as permitted sender) Received: from [74.125.82.170] (HELO mail-wy0-f170.google.com) (74.125.82.170) by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 15 Jun 2010 13:05:24 +0000 Received: by wyf22 with SMTP id 22so2532507wyf.29 for ; Tue, 15 Jun 2010 06:05:04 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:mime-version:received:received:in-reply-to :references:date:message-id:subject:from:to:content-type; bh=uj1svCa22XMVKiTZnb2kphZthRZ2tLsnu9AqWLZFKgg=; b=iiFob5Bvuvkktk0Hrs62IQD5hJcE/xBUhWRcpmo5Q9wF6LLuiBk9tdBVkEZ7OIKlEm JRWSet0doH9pZaSCx9UbMTMruUeuN99CH1qSpCxJ9kY0tAuk8rREppy5gFDE9PUXKj32 vOK5HfyvrKI9ieabzk6QwnDcAdORobyYFRw4E= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :content-type; b=X+f3nwovlrzp1U1uqh6LSvcLZaG3mZ7ko/kwnyc/fBMbq/+78wN6KfhD/AALnIMzji 7TD+ZAX5t75oaoQHbF4bcuQ6lfdeDDFWNkg0uCOAAfkFUioGDzKogUWGNfBZ0ED29lG9 uK+h/UXIH7Q3ZMh17ZWEhFOMROjUt6I92vjpU= MIME-Version: 1.0 Received: by 10.227.141.137 with SMTP id m9mr7216283wbu.202.1276607104090; Tue, 15 Jun 2010 06:05:04 -0700 (PDT) Received: by 10.227.148.16 with HTTP; Tue, 15 Jun 2010 06:05:04 -0700 (PDT) In-Reply-To: <4C16BC9A.4040607@gmail.com> References: <4C16BC9A.4040607@gmail.com> Date: Tue, 15 Jun 2010 15:05:04 +0200 Message-ID: Subject: Re: Issues or no issue... From: "Md. Jahid Shohel" To: dev@click.apache.org Content-Type: multipart/alternative; boundary=0016e6585edaf505df0489114008 X-Virus-Checked: Checked by ClamAV on apache.org --0016e6585edaf505df0489114008 Content-Type: text/plain; charset=ISO-8859-1 Hi Bob, Thanks for your reply. On Tue, Jun 15, 2010 at 1:34 AM, Bob Schellink wrote: > Hi Jahid, > > On 15/06/2010 02:33, Md. Jahid Shohel wrote: > > > > > 2) The same way DeployUtils class is package local, and I made a true > copy. > > > I assume you needed this for #3? See my next comment. > > > > 3) The org.apache.Control interface was made to be both "a generic > > control" and "a deploy aware control". Which I felt more of mixing of > > concern. Shouldn't these be two separate interfaces. Like - > > > > public interface DeployAwareControl{ > > public void onDeploy(ServletContext context); > > } > > > > And for backward compatibility we can make Control interface extending > > DeployAwareControl. > > > Click provides two ways of deploying resources. Control#onDeploy and the > Servlet3.0 approach of > packaging resources under /META-INF/resources. I think Control#onDeploy > should be deprecated as it > has some major drawbacks: > > - Control has to be instantiated during startup in order to invoke > onDeploy. At startup time there > is no request, and thus no Context object. The net effect of this is that > the Control cannot rely on > Context in it's constructor. Accessing Context in the Control constructor > is useful because one can > pull data from the Session to retrieve state. > - changes to the resources have to be updated in two places, once for > getHeadElements and again for > onDeploy > > > I still see onDeploy has more potential then making it deprecated. There are lots of things that developer can do when the application is deploying. I can give one example, the spring integration I have written (more details can be found on http://code.google.com/p/doubleclick/wiki/clickspring) loads up the spring context during the deploy time - This is just an example that what we can do on deploy. There are many other things we can do, we can prepare services common services which need to intercept each call without writing a filter, we can prepare services (spring, JPA, Hibernate) which will be singleton service through out the application. And since its singleton and so the only perfect place we can load up the singleton service is onDeploy. But I totally agree with you, that this does not need to be a Control. We can just make a separate one. > > 4) The "isTemplate" method is having a bug. In the method we are not > > checking if the file is really a file or folder, Because on my Linux > > (Ubuntu) I was able to create a folder with .htm or .jsp extension. I > > know this might not happen so often, but still I think it might create > > problem. > > > isTemplate is used at runtime and would create overhead if we were to touch > the filesystem. I don't > see a use case for having .htm and .jsp named folders inside the webapp. > That said what is the > behavior of having such a folder defined? > > > I agree with you, that there is no use case which can make .htm or .jsp named folders. But logic wise it did not seems correct that we do not check if its a file or folder. Any naughty developer can make a .htm folder inside the / and that .htm folder will still be processed on deploy time, and will try to figure out its variables and auto bind them. > > 6) When the application is loading, we load all page classes, and their > > bindable page fields. Could be a memory issue when benchmarking Click > > against other frameworks. We can load bindable page fields on demand, > > and cache them. That way we will not end up loading all page fields even > > if its not needed. For an application with 100 pages this could be a > > performance issue. As an example: say on http://click.apache.org there > > are lots of pages which is not browsed by user (example license content > > page, how to release, and pages like that), those pages will still be > > loaded. I know an application running for long time might be loading all > > pages eventually. But still, we can load them on demand, and we can > > cache loaded classes. And we can also keep the house keeping thread to > > eventually clear up rarely loaded pages. > > > I'd like to see lazy loading as well so feel free to open a JIRA for this. > Not only does it affect > startup but it will allow Click to run in production mode under GAE[1]. GAE > does not support the > method servletContext.getResourcePaths which XmlConfigService uses > internally. Lazy loading does > present its own problems such as synchronizing the data structures used to > store page classes and > templates. > > I will create one JIRA issue for this, and I will also add patch about how I see the lazy loading should work for this (which will include synchronizing the data structure used to store page classes and templates. > > > 7) On "getBindablePageFields" method of XmlConfigService, this piece of > > code - > > > > Field[] publicFields = pageClass.getFields(); > > for (Field field : publicFields) { > > fieldMap.put(field.getName(), field); > > } > > > > Though @Bindable fields are still processed, but here we again we are > > processing all public fields (even though they have @Bindable and > > already processed). Also there are some repetition of code (on if-else) > > block. Possible to improve. Will help to get a better picture when > > benchmarking. > > > What optimization do you have in mind? > > I will reply details about this one, I have to look into it to find out where did I see that. > Kind regards > > Bob > > [1]: https://issues.apache.org/jira/browse/CLK-639 > Regards, Jahid --0016e6585edaf505df0489114008 Content-Type: text/html; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Hi Bob,

Thanks for your reply.

On Tue, Jun 15, 2010 at 1:34 A= M, Bob Schellink <= sabob1@gmail.com> wrote:
Hi Jahid,

On 15/06/2010 02:33, Md. Jahid Shohel wrote:

>
> 2) The same way DeployUtils class is package local, and I made a true = copy.


I assume you needed this for #3? See my next comment.


> 3) The org.apache.Control interface was made to be both "a generi= c
> control" and "a deploy aware control". Which I felt mor= e of mixing of
> concern. Shouldn't these be two separate interfaces. Like -
>
> public interface DeployAwareControl{
> =A0 =A0 =A0public void onDeploy(ServletContext context);
> }
>
> And for backward compatibility we can make Control interface extending=
> DeployAwareControl.


Click provides two ways of deploying resources. Control#onDeploy and = the Servlet3.0 approach of
packaging resources under /META-INF/resources. I think Control#onDeploy sho= uld be deprecated as it
has some major drawbacks:

=A0- Control has to be instantiated during startup in order to invoke onDep= loy. At startup time there
is no request, and thus no Context object. The net effect of this is that t= he Control cannot rely on
Context in it's constructor. Accessing Context in the Control construct= or is useful because one can
pull data from the Session to retrieve state.
=A0- changes to the resources have to be updated in two places, once for ge= tHeadElements and again for
onDeploy


I still see onDeploy has more potential then ma= king it deprecated. There are lots of things that developer can do when the application is=20 deploying. I can give one example, the spring integration I have written (more details can be found on=20 http://co= de.google.com/p/doubleclick/wiki/clickspring) loads up the=20 spring context during the deploy time -

<=
span class=3D"tag"><controls>
=A0 =A0 <=
/span><control classname=3D"org.apache.click.doubleclick.injectspring.SpringControl"=
;/>

</controls>
This is just an example that what we can do on deploy. There are many other=20 things we can do, we can prepare services common services which need to=20 intercept each call without writing a filter, we can prepare services=20 (spring, JPA, Hibernate) which will be singleton service through out the application. And since its singleton and so the only perfect place we=20 can load up the singleton service is onDeploy. But I totally agree with=20 you, that this does not need to be a Control. We can just make a=20 separate one.


=A0
> 4) The "isTemplate" method is having a bug. In the method we= are not
> checking if the file is really a file or folder, Because on my Linux > (Ubuntu) I was able to create a folder with .htm =A0or .jsp extension.= I
> know this might not happen so often, but still I think it might create=
> problem.


isTemplate is used at runtime and would create overhead if we were to= touch the filesystem. I don't
see a use case for having .htm and .jsp named folders inside the webapp. Th= at said what is the
behavior of having such a folder defined?


I agree with you, that there is no use case whi= ch can make .htm or .jsp named folders. But logic wise it did not seems cor= rect that we do not check if its a file or folder. Any naughty developer ca= n make a .htm folder inside the / and that .htm folder will still be proces= sed on deploy time, and will try to figure out its variables and auto bind = them.

=A0
=
> 6) When the application is loading, we load all page classes, and thei= r
> bindable page fields. Could be a memory issue when benchmarking Click<= br> > against other frameworks. We can load bindable page fields on demand,<= br> > and cache them. That way we will not end up loading all page fields ev= en
> if its not needed. For an application with 100 pages this could be a > performance issue. As an example: say on http://click.apache.org there
> are lots of pages which is not browsed by user (example license conten= t
> page, how to release, and pages like that), those pages will still be<= br> > loaded. I know an application running for long time might be loading a= ll
> pages eventually. But still, we can load them on demand, and we can > cache loaded classes. And we can also keep the house keeping thread to=
> eventually clear up rarely loaded pages.


I'd like to see lazy loading as well so feel free to open a JIRA = for this. Not only does it affect
startup but it will allow Click to run in production mode under GAE[1]. GAE= does not support the
method servletContext.getResourcePaths which XmlConfigService uses internal= ly. Lazy loading does
present its own problems such as synchronizing the data structures used to = store page classes and
templates.

I will create one JIRA issue = for this, and I will also add patch about how I see the lazy loading should= work for this (which will include synchronizing the data structure used to= store page classes and templates.
=A0

> 7) On "getBindablePageFields" method of XmlConfigService, th= is piece of
> code -
>
> Field[] publicFields =3D pageClass.getFields();
> =A0 =A0 for (Field field : publicFields) {
> =A0 =A0 =A0 =A0 fieldMap.put(field.getName(), field);
> }
>
> Though @Bindable fields are still processed, but here we again we are<= br> > processing all public fields (even though they have @Bindable and
> already processed). Also there are some repetition of code (on if-else= )
> block. Possible to improve. Will help to get a better picture when
> benchmarking.


What optimization do you have in mind?


I will reply details about this one, I have to lo= ok into it to find out where did I see that.


Kind regards

Bob

[1]: https://issues.apache.org/jira/browse/CLK-639


Regards,


Jahid
--0016e6585edaf505df0489114008--