click-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Finn Bock <>
Subject Re: Issues or no issue...
Date Mon, 14 Jun 2010 20:01:37 GMT
2010/6/14 Md. Jahid Shohel

> Hi Guys,
> While I was working on click-annotation project (which I have sent email
> earlier on both user and dev group). I figured out couple of things. These
> could be issues that need to be taken care off, or may be they are not
> issues at all. I was not sure that's why I did not create any jira issue. If
> Click expert guys verifies and agrees that some of them should be jira
> issue, then I can raise the issue on jira. Issues are -
> 1) Private methods on XmlConfigService. While I was trying to integrate the
> annotation processing stuffs, I figured out that the methods on
> XmlConfigService is private/package local scope. Which strictly prohibits
> the scope of extending the class to add more custom behavior. So, I had made
> a true copy and then modify the class (made methods protected and splitted
> common behavior). It will be really painful to update the true copy for
> upcoming Click releases.

In jira [1] and the development notes for 2.3 [2] there is outlined a
desire for a default implementation of the ConfigService. I'm working
on this and will welcome your comments when it is ready. Your Click
annotations is an obvious candidate for subclassing the
DefaultConfigService class instead of XmlConfigService.

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

Except for the license header. Naughty :-)

> 3) ...

> 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.

Please raise a jira.

> 5) The entire code on XmlConfigService is not Java 1.5 compatible. Some of
> it uses generics, and some of it does not.

At the moment the API have been converted to generics [3], [4]. There
is still work to done for the internal code, and yes, the
XmlConfigService class is especially lacking in this regard.

> 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 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.

My gut feeling says that this does not matter at all and that Click
does the right thing, but if you have seen the preloading causing
problems or benchmark numbers that shows this to be a problem, please
create a jira.

> 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.

Again, I doubt you can make any noticeable performance improvement to
this code, but I'm happy to be proven wrong.

> My English is not much good, and I did not have time to review the mail
> before sending. So for any mistake, I apology. And if you do not understand
> something, let me know, I will explain more about that.



View raw message