Mr Chung,
You mentioned three areas where the code could be improved.
On Thu, 16 May 2002, Kin-Man Chung wrote:
> 1. I notice the following code pattern that is now generated.
>
> bitmask.set(1);
> addTagToVector(tags, 1, new Integer(_jspx_eval_eg_foo_0));
> if (_jspx_eval_eg_foo_0 != javax.servlet.jsp.tagext.Tag.EVAL_BODY_INCLUD
> E) {
> out = pageContext.pushBody();
> _jspx_th_eg_foo_0.setBodyContent((javax.servlet.jsp.tagext.BodyContent
> ) out);
> _jspx_th_eg_foo_0.doInitBody();
> }
>
> and from finallies:
>
> if (bitmask.get(1)) {
> if (((Integer)tags.elementAt(1)).intValue() != javax.servlet.jsp.tagext.Ta
> g.EVAL_BODY_INCLUDE)
> out = pageContext.popBody();
> }
>
> I notice that the code "bitmask.set(1);" is there just for popBody, so
> if we move it to inside the test that do pushBody, then we don't need
> to do the test in the finallies. See the codes below.
>
> addTagToVector(tags, 1, new Integer(_jspx_eval_eg_foo_0));
> if (_jspx_eval_eg_foo_0 != javax.servlet.jsp.tagext.Tag.EVAL_BODY_INCLUD
> E) {
> bitmask.set(1);
> out = pageContext.pushBody();
> _jspx_th_eg_foo_0.setBodyContent((javax.servlet.jsp.tagext.BodyContent
> ) out);
> _jspx_th_eg_foo_0.doInitBody();
> }
>
> and in finallies:
>
> if (bitmask.get(1)) {
> out = pageContext.popBody();
> }
>
> Maybe you don't need to change the argument for addTagToVector afterall!
You're right, in fact, we might get rid of the addTagToVector completely with
your next two proposals.
> 2. We don't really need to call finallies if there is no exceptions, so
> the call to finallies can be placed in a catch block instead of a finally
> block. This would save us time to check for all the bits that have been
> cleared, under normal execution.
Yes, this is a very good point.
> Now you mentioned the use of an array to hold tag objects, I have
> another idea. Why don't we use a stack to simulate the runtime state?
> Each stack entry would have a tag object and a state. State 0 means
> call release() only, and state 1 means call popBody() and then release().
> We push an entry onto the stack when we enter a tag body, and pop it
> when we exit the body. We change the state on the stack top when we do
> a pushBody. When an exception is thrown, we just need to pop the entries
> from the stack to decide what to do. For efficiency, we can use an
> array to simulate the stack, and its size would be the number of nesting
> of the tags.
>
> I think this is quite simple and faster. What do you think?
Yes! Yes!
In my last e-mail, I mentionned that there is only two actions that we need
to do in the "finallies". This is:
1) do a release() on a Tag
2) do a popBody()
We could push on the stack in this order:
Either:
1- A tag
2- The Action identifier for release (a Constant representing "release()")
Or:
1- The Action identifier for popBody (a Constant representing "popBody()")
Now the "finallies" code look like this:
While stack is not empty:
Pop an action
If action is RELEASE then
pop a Tag
Do a Tag.release()
Otherwise /* Action is assumed to be POPBODY */
do a popBody()
So the "finallies" could very well be "inlined" because it would be short.
The other modification would be when we begin the "pseudo" finally block. At
that point we have to remove from the stack what was pushed at the begin of
the "pseudo" try block. If we do a popBody() in the "finally", then we must
pop one element. If we do a release(), we will pop two elements.
In the event of an exception anywhere in the page, the stack would contain
only the actions that must be performed in the order to be performed. Your
solution is clean, efficient and simple!
The stack could very well be an Object[], but if we must rely on arrayCopy(),
to extend the array periodically, would it be really much more efficient than
using a plain Stack object? After all, this is the way a Stack is implemented
by the JDK itself. On the other hand, we know the lower and upper bounds of the
stack. Each tag will need to be pushed at least once for the "release()" and
the action identifier will need to be pushed too (so we push two times the
number of Tags in the page). And maybe we will need to push once more for a
"popBody()" (so one more time to indicate the action).
This would give us an array of size 2 to 3 times the number of tags in the page.
I think it would be more efficient to allocate an array of 3 times the number of
tags and save us the occasional overhead of arrayCopy() calls to extend the array.
Now, I see two ways to determine the size of the array. Either, after
generating the _jsp_service() method, the class Generator could produce the
declaration of the array because at this time, it would know what was the
"upper" index in the array. But it would not be very elegant. The variables in
the generated class are expected to be put at the top of the generated java
file, not in the middle between method declarations.
The other option would be to be able to retrieve the number of tags in the page
from the PageInfo object. This would be cleaner, because we could then declare
the array at the top of the generated java file.
In a word I'm very much willing to produce such a patch. But, we have to think
if now is the appropriate time. Jasper2 is in the process of being stabilized,
shouldn't we wait for the current bugs to be fixed, and once a stable release
is ready for Tomcat, start an optimisation phase? This is your call, if you
want it now, I could have such a patch ready for monday for your approval.
Thanks!
--
Denis Benoit
[EMAIL PROTECTED]
--
To unsubscribe, e-mail: <mailto:[EMAIL PROTECTED]>
For additional commands, e-mail: <mailto:[EMAIL PROTECTED]>
