Skip to content

Commit d929bec

Browse files
committed
When generating JSP runtime error messages that quote the relevant JSP source code, switch from using the results of the JSP page parsing process to using the JSR 045 source map data to identify the correct part of the JSP source from the stack trace. This significantly reduces the memory footprint of Jasper in development mode, provides a small performance improvement for error page generation and enables source quotes to continue to be provided after a Tomcat restart. Commit: apache/tomcat@93ae8bd Signed-off-by: Flavia Rainone <frainone@redhat.com>
1 parent 70ef46f commit d929bec

10 files changed

Lines changed: 380 additions & 261 deletions

File tree

src/main/java/org/apache/jasper/JspCompilationContext.java

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -510,7 +510,7 @@ public void setPrototypeMode(boolean pm) {
510510
}
511511

512512
/**
513-
* Package name for the generated class is make up of the base package
513+
* Package name for the generated class is made up of the base package
514514
* name, which is user settable, and the derived package name. The
515515
* derived package name directly mirrors the file hierarchy of the JSP page.
516516
* @return the package name
@@ -542,12 +542,20 @@ protected String getDerivedPackageName() {
542542
return derivedPackageName;
543543
}
544544

545+
/**
546+
* @return The base package name into which all servlet and associated code
547+
* is generated
548+
*/
549+
public String getBasePackageName() {
550+
return basePackageName;
551+
}
552+
545553
/**
546554
* The package name into which the servlet class is generated.
547-
* @param servletPackageName The package name to use
555+
* @param basePackageName The package name to use
548556
*/
549-
public void setServletPackageName(String servletPackageName) {
550-
this.basePackageName = servletPackageName;
557+
public void setBasePackageName(String basePackageName) {
558+
this.basePackageName = basePackageName;
551559
}
552560

553561
/**

src/main/java/org/apache/jasper/compiler/Compiler.java

Lines changed: 32 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -81,26 +81,30 @@ public void init(JspCompilationContext ctxt, JspServletWrapper jsw) {
8181

8282
// --------------------------------------------------------- Public Methods
8383

84-
/**
85-
* <p>
86-
* Retrieves the parsed nodes of the JSP page, if they are available. May
87-
* return null. Used in development mode for generating detailed error
88-
* messages. http://bz.apache.org/bugzilla/show_bug.cgi?id=37062.
89-
* </p>
90-
* @return the page nodes
91-
*/
92-
public Node.Nodes getPageNodes() {
93-
return this.pageNodes;
84+
public SmapStratum getSmap(String className) {
85+
86+
Map<String,SmapStratum> smaps = ctxt.getRuntimeContext().getSmaps();
87+
SmapStratum smap = smaps.get(className);
88+
89+
if (smap == null && !options.isSmapSuppressed()) {
90+
// Tomcat was restarted so cached SMAP has been lost. However, it
91+
// was written to the class file so it can be recovered.
92+
smap = SmapUtil.loadSmap(className, ctxt.getJspLoader());
93+
if (smap != null) {
94+
smaps.put(className, smap);
95+
}
96+
}
97+
98+
return smap;
9499
}
95100

101+
96102
/**
97103
* Compile the jsp file into equivalent servlet in .java file
98104
*
99-
* @return a smap for the current JSP page, if one is generated, null
100-
* otherwise
101105
* @throws Exception Error generating Java source
102106
*/
103-
protected String[] generateJava() throws Exception {
107+
protected Map<String,SmapStratum> generateJava() throws Exception {
104108

105109
String[] smapStr = null;
106110

@@ -216,7 +220,6 @@ protected String[] generateJava() throws Exception {
216220
// generate prototype .java file for the tag file
217221
try (ServletWriter writer = setupContextWriter(javaFileName)) {
218222
Generator.generate(writer, this, pageNodes);
219-
return null;
220223
}
221224
}
222225

@@ -280,9 +283,14 @@ protected String[] generateJava() throws Exception {
280283
throw e;
281284
}
282285

286+
Map<String,SmapStratum> smaps = null;
287+
283288
// JSR45 Support
284289
if (!options.isSmapSuppressed()) {
285-
smapStr = SmapUtil.generateSmap(ctxt, pageNodes);
290+
smaps = SmapUtil.generateSmap(ctxt, pageNodes);
291+
// Add them to the web application wide cache for future lookup in
292+
// error handling etc.
293+
ctxt.getRuntimeContext().getSmaps().putAll(smaps);
286294
}
287295

288296
// If any proto type .java and .class files was generated,
@@ -292,7 +300,7 @@ protected String[] generateJava() throws Exception {
292300
// generate .class again from the new .java file just generated.
293301
tfp.removeProtoTypeFiles(ctxt.getClassFileName());
294302

295-
return smapStr;
303+
return smaps;
296304
}
297305

298306
private ServletWriter setupContextWriter(String javaFileName)
@@ -317,12 +325,15 @@ private ServletWriter setupContextWriter(String javaFileName)
317325
/**
318326
* Servlet compilation. This compiles the generated sources into
319327
* Servlets.
320-
* @param smap The SMAP files for source debugging
328+
*
329+
* @param smaps The source maps for the class(es) generated from the source
330+
* file
331+
*
321332
* @throws FileNotFoundException Source files not found
322333
* @throws JasperException Compilation error
323334
* @throws Exception Some other error
324335
*/
325-
protected abstract void generateClass(String[] smap)
336+
protected abstract void generateClass(Map<String,SmapStratum> smaps)
326337
throws FileNotFoundException, JasperException, Exception;
327338

328339
/**
@@ -372,12 +383,12 @@ public void compile(boolean compileClass, boolean jspcMode)
372383
}
373384

374385
try {
375-
String[] smap = generateJava();
386+
Map<String,SmapStratum> smaps = generateJava();
376387
File javaFile = new File(ctxt.getServletJavaFileName());
377388
Long jspLastModified = ctxt.getLastModified(ctxt.getJspFile());
378389
javaFile.setLastModified(jspLastModified.longValue());
379390
if (compileClass) {
380-
generateClass(smap);
391+
generateClass(smaps);
381392
// Fix for bugzilla 41606
382393
// Set JspServletWrapper.servletClassLastModifiedTime after successful compile
383394
String targetFileName = ctxt.getClassFileName();
@@ -404,13 +415,7 @@ public void compile(boolean compileClass, boolean jspcMode)
404415
errDispatcher = null;
405416
pageInfo = null;
406417

407-
// Only get rid of the pageNodes if in production.
408-
// In development mode, they are used for detailed
409-
// error messages.
410-
// http://bz.apache.org/bugzilla/show_bug.cgi?id=37062
411-
if (!this.options.getDevelopment()) {
412-
pageNodes = null;
413-
}
418+
pageNodes = null;
414419

415420
if (ctxt.getWriter() != null) {
416421
ctxt.getWriter().close();

src/main/java/org/apache/jasper/compiler/JDTCompiler.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ public class JDTCompiler extends org.apache.jasper.compiler.Compiler {
7272
* Compile the servlet from .java file to .class file
7373
*/
7474
@Override
75-
protected void generateClass(String[] smap)
75+
protected void generateClass(Map<String,SmapStratum> smaps)
7676
throws FileNotFoundException, JasperException, Exception {
7777

7878
long t1 = 0;
@@ -561,7 +561,7 @@ public void acceptResult(CompilationResult result) {
561561

562562
// JSR45 Support
563563
if (! options.isSmapSuppressed()) {
564-
SmapUtil.installSmap(smap);
564+
SmapUtil.installSmap(smaps);
565565
}
566566
}
567567
}

src/main/java/org/apache/jasper/compiler/JspRuntimeContext.java

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -198,6 +198,14 @@ public JspRuntimeContext(ServletContext context, Options options) {
198198
*/
199199
private FastRemovalDequeue<JspServletWrapper> jspQueue = null;
200200

201+
/**
202+
* Map of class name to associated source map. This is maintained here as
203+
* multiple JSPs can depend on the same file (included JSP, tag file, etc.)
204+
* so a web application scoped Map is required.
205+
*/
206+
private final Map<String,SmapStratum> smaps = new ConcurrentHashMap<>();
207+
208+
201209
// ------------------------------------------------------ Public Methods
202210

203211
/**
@@ -417,9 +425,12 @@ public long getLastJspQueueUpdate() {
417425
return lastJspQueueUpdate;
418426
}
419427

428+
public Map<String,SmapStratum> getSmaps() {
429+
return smaps;
430+
}
420431

421-
// -------------------------------------------------------- Private Methods
422432

433+
// -------------------------------------------------------- Private Methods
423434

424435
/**
425436
* Method used to initialize classpath for compiles.

src/main/java/org/apache/jasper/compiler/SmapGenerator.java

Lines changed: 0 additions & 119 deletions
This file was deleted.
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one or more
3+
* contributor license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright ownership.
5+
* The ASF licenses this file to You under the Apache License, Version 2.0
6+
* (the "License"); you may not use this file except in compliance with
7+
* the License. You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*/
17+
package org.apache.jasper.compiler;
18+
19+
public class SmapInput {
20+
21+
private final String fileName;
22+
private final int lineNumber;
23+
24+
25+
public SmapInput(String fileName, int lineNumber) {
26+
this.fileName = fileName;
27+
this.lineNumber = lineNumber;
28+
}
29+
30+
31+
public String getFileName() {
32+
return fileName;
33+
}
34+
35+
36+
public int getLineNumber() {
37+
return lineNumber;
38+
}
39+
}

0 commit comments

Comments
 (0)