[Bug 53869] Performance tuning solution to resolve too many cascaded JspContextWrapper issue

2013-01-25 Thread bugzilla
https://issues.apache.org/bugzilla/show_bug.cgi?id=53869

--- Comment #11 from Mark Thomas ma...@apache.org ---
The test case used to evaluate this patch is flawed. It fails to take account
of the call to rootJspCtxt.getELContext() and does not compare the performance
of the new implementation for a simple JSP page that does not use EL and does
not have nested tags. When these additional issues are taken into consideration
the proposed patch significantly impacts performance for these simple pages.
Performance (as measured by this test case) is 1.5 to 2 times slower for simple
pages.

However, it is not all doom and gloom. A small tweak to the proposed patch to
use lazy instantiation for the EL context and performance is (as near as makes
no difference) back to where it was for simple pages and still noticeably
improved for the more complex case.

-- 
You are receiving this mail because:
You are the assignee for the bug.

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



[Bug 53869] Performance tuning solution to resolve too many cascaded JspContextWrapper issue

2013-01-25 Thread bugzilla
https://issues.apache.org/bugzilla/show_bug.cgi?id=53869

Mark Thomas ma...@apache.org changed:

   What|Removed |Added

 Status|NEW |RESOLVED
 Resolution|--- |FIXED

--- Comment #12 from Mark Thomas ma...@apache.org ---
The modified patch has been applied to trunk and will be included in 7.0.36
onwards.

-- 
You are receiving this mail because:
You are the assignee for the bug.

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



[Bug 53869] Performance tuning solution to resolve too many cascaded JspContextWrapper issue

2012-12-18 Thread bugzilla
https://issues.apache.org/bugzilla/show_bug.cgi?id=53869

Sheldon Shao xs...@ebay.com changed:

   What|Removed |Added

 Status|NEEDINFO|NEW

-- 
You are receiving this mail because:
You are the assignee for the bug.

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



[Bug 53869] Performance tuning solution to resolve too many cascaded JspContextWrapper issue

2012-11-21 Thread bugzilla
https://issues.apache.org/bugzilla/show_bug.cgi?id=53869

--- Comment #10 from Sheldon Shao xs...@ebay.com ---
Actually, There are 8-level tag files cascaded in our page. You can refer this
screenshot.
In this screenshot. Index.jsp was hit 80 times.  It has almost 1 ELs in
runtime. In this situation, JspContextWrapper.findAttribute becomes a big one
of EL evaluation.

-- 
You are receiving this mail because:
You are the assignee for the bug.

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



[Bug 53869] Performance tuning solution to resolve too many cascaded JspContextWrapper issue

2012-11-18 Thread bugzilla
https://issues.apache.org/bugzilla/show_bug.cgi?id=53869

--- Comment #9 from Sheldon Shao xs...@ebay.com ---
Created attachment 29604
  -- https://issues.apache.org/bugzilla/attachment.cgi?id=29604action=edit
Screenshot from JProfiler

Actually, There are 8-level tag files cascaded in our page. You can refer this
screenshot.
In this screenshot. Index.jsp was hit 80 times.  It has almost 1 ELs in
runtime. In this situation, JspContextWrapper.findAttribute becomes a big one
of EL evaluation.

-- 
You are receiving this mail because:
You are the assignee for the bug.

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



[Bug 53869] Performance tuning solution to resolve too many cascaded JspContextWrapper issue

2012-11-12 Thread bugzilla
https://issues.apache.org/bugzilla/show_bug.cgi?id=53869

--- Comment #7 from Konstantin Kolinko knst.koli...@gmail.com ---
1. This change in JspContextWrapper constructor means that it will create EL
context regardless of whether it will be used or not:

+ this.elContext = rootJspCtxt.getELContext();

See PageContextImpl.getELContext(),
JspApplicationContextImpl.createELContext(..).

The EL Context creation operation involves using a PrivilegedAction and firing
an event through listeners. It is not cheap.

I think this change in behaviour is undesireable.


2. I do not expect much from your proposed optimization. Using a delegation is
a common pattern. Replacing 4 chained redirects with 1 redirect is not much of
a change.

3. If someone extends JspContextWrapper, its behaviour will be broken by this
change. (Is it a concern? Would anyone extend it? Isn't it Jasper internals?)

-- 
You are receiving this mail because:
You are the assignee for the bug.

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



[Bug 53869] Performance tuning solution to resolve too many cascaded JspContextWrapper issue

2012-11-12 Thread bugzilla
https://issues.apache.org/bugzilla/show_bug.cgi?id=53869

--- Comment #8 from Mark Thomas ma...@apache.org ---
To get performance improvements above has required 5 levels of nesting and 10^7
iterations. That works out to around 50 nanoseconds per request.

I am far from convinced that the test case used to justify this changes
represents a typical use case.

I share Konstantin's concerns regarding the ELContext.

I do not believe folks would be extending JspContextWrapper.

Without a better justification for this change, I am leaning towards WONTFIX.

-- 
You are receiving this mail because:
You are the assignee for the bug.

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



[Bug 53869] Performance tuning solution to resolve too many cascaded JspContextWrapper issue

2012-11-11 Thread bugzilla
https://issues.apache.org/bugzilla/show_bug.cgi?id=53869

--- Comment #4 from Sheldon Shao xs...@ebay.com ---
Created attachment 29586
  -- https://issues.apache.org/bugzilla/attachment.cgi?id=29586action=edit
Old JspContextWrapper

-- 
You are receiving this mail because:
You are the assignee for the bug.

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



[Bug 53869] Performance tuning solution to resolve too many cascaded JspContextWrapper issue

2012-11-11 Thread bugzilla
https://issues.apache.org/bugzilla/show_bug.cgi?id=53869

--- Comment #5 from Sheldon Shao xs...@ebay.com ---
Created attachment 29587
  -- https://issues.apache.org/bugzilla/attachment.cgi?id=29587action=edit
Test case comparison for performance tuning

-- 
You are receiving this mail because:
You are the assignee for the bug.

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



[Bug 53869] Performance tuning solution to resolve too many cascaded JspContextWrapper issue

2012-11-11 Thread bugzilla
https://issues.apache.org/bugzilla/show_bug.cgi?id=53869

Sheldon Shao xs...@ebay.com changed:

   What|Removed |Added

  Attachment #29586|application/octet-stream|text/plain
  mime type||

-- 
You are receiving this mail because:
You are the assignee for the bug.

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



[Bug 53869] Performance tuning solution to resolve too many cascaded JspContextWrapper issue

2012-11-11 Thread bugzilla
https://issues.apache.org/bugzilla/show_bug.cgi?id=53869

Sheldon Shao xs...@ebay.com changed:

   What|Removed |Added

  Attachment #29587|application/octet-stream|text/plain
  mime type||

-- 
You are receiving this mail because:
You are the assignee for the bug.

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



[Bug 53869] Performance tuning solution to resolve too many cascaded JspContextWrapper issue

2012-11-11 Thread bugzilla
https://issues.apache.org/bugzilla/show_bug.cgi?id=53869

--- Comment #6 from Sheldon Shao xs...@ebay.com ---
Hi Mark.

I added the test case.
Here is a result of this case:

Duration:422
DurationOld:748
Duration:203
DurationOld:717
Duration:187
DurationOld:702
Duration:187
DurationOld:686

-- 
You are receiving this mail because:
You are the assignee for the bug.

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



[Bug 53869] Performance tuning solution to resolve too many cascaded JspContextWrapper issue

2012-10-28 Thread bugzilla
https://issues.apache.org/bugzilla/show_bug.cgi?id=53869

Mark Thomas ma...@apache.org changed:

   What|Removed |Added

 Status|NEW |NEEDINFO

--- Comment #3 from Mark Thomas ma...@apache.org ---
Please provide a test case that demonstrates the difference. The test case
doesn't need to compare the old and new. As long as it generates timing
information for the current implementation then both implementations may be
tested in turn.

-- 
You are receiving this mail because:
You are the assignee for the bug.

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



[Bug 53869] Performance tuning solution to resolve too many cascaded JspContextWrapper issue

2012-09-18 Thread bugzilla
https://issues.apache.org/bugzilla/show_bug.cgi?id=53869

Jarek Gawor jga...@gmail.com changed:

   What|Removed |Added

 OS||All

--- Comment #2 from Jarek Gawor jga...@gmail.com ---
Can somebody please take a look at this patch?

-- 
You are receiving this mail because:
You are the assignee for the bug.

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



[Bug 53869] Performance tuning solution to resolve too many cascaded JspContextWrapper issue

2012-09-18 Thread bugzilla
https://issues.apache.org/bugzilla/show_bug.cgi?id=53869

Jarek Gawor jga...@gmail.com changed:

   What|Removed |Added

 CC||jga...@gmail.com

-- 
You are receiving this mail because:
You are the assignee for the bug.

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



[Bug 53869] Performance tuning solution to resolve too many cascaded JspContextWrapper issue

2012-09-13 Thread bugzilla
https://issues.apache.org/bugzilla/show_bug.cgi?id=53869

--- Comment #1 from Sheldon Shao xs...@ebay.com ---
Created attachment 29373
  -- https://issues.apache.org/bugzilla/attachment.cgi?id=29373action=edit
Time taken percentage from one sample page

-- 
You are receiving this mail because:
You are the assignee for the bug.

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



[Bug 53869] Performance tuning solution to resolve too many cascaded JspContextWrapper issue

2012-09-13 Thread bugzilla
https://issues.apache.org/bugzilla/show_bug.cgi?id=53869

Sheldon Shao xs...@ebay.com changed:

   What|Removed |Added

  Attachment #29372|0   |1
   is patch||

-- 
You are receiving this mail because:
You are the assignee for the bug.

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