Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Failed to compile template: constant string too long #25

Closed
tomholub opened this issue Oct 30, 2020 · 8 comments · Fixed by #26
Closed

Failed to compile template: constant string too long #25

tomholub opened this issue Oct 30, 2020 · 8 comments · Fixed by #26
Assignees
Labels
bug Something isn't working

Comments

@tomholub
Copy link

Steps to reproduce:

  • a long file in tag/something.jte
  • include in another jte file with @tag.something()
  • render

Long templates should be somehow split up to stay under the Java length limit.

Failed to compile template, error at tag/content/samples.jte:6156
<snip>/jte-classes/gg/jte/generated/tag/content/JtesamplesGenerated.java:6: error: constant string too long
        jteOutput.writeContent("\n <div class=\"row\">\n ...................");
         ^\n1 error
\n\n\tat gg.jte.internal.ClassFilesCompiler.runCompiler(ClassFilesCompiler.java:35)
\n\tat gg.jte.internal.ClassFilesCompiler.compile(ClassFilesCompiler.java:23)
\n\tat gg.jte.internal.TemplateCompiler.precompile(TemplateCompiler.java:77)
\n\tat gg.jte.internal.TemplateCompiler.load(TemplateCompiler.java:40)
\n\tat gg.jte.TemplateEngine.resolveTemplate(TemplateEngine.java:267)
\n\tat gg.jte.TemplateEngine.render(TemplateEngine.java:151)
\n\tat io.javalin.plugin.rendering.template.JavalinJte.render(JavalinJte.kt:33)
\n\tat io.javalin.plugin.rendering.JavalinRenderer.renderBasedOnExtension(JavalinRenderer.kt:34)
\n\tat io.javalin.http.Context.render(Context.kt:480)\n\tat com.flowcrypt.server.controller.webapp.TestPanelKt.testPanel(TestPanel.kt:16)
\n\tat com.flowcrypt.server.WebApp$4.invoke(WebApp.kt:37)
\n\tat com.flowcrypt.server.WebApp$4.invoke(WebApp.kt:20)
\n\tat com.flowcrypt.server.WebApp$sam$io_javalin_http_Handler$0.handle(WebApp.kt)
\n\tat com.flowcrypt.commands.Server$getAccessManager$1.manage(Server.kt:120)
\n\tat io.javalin.http.JavalinServlet$addHandler$protectedHandler$1.handle(JavalinServlet.kt:121)
\n\tat io.javalin.http.JavalinServlet$service$2$1.invoke(JavalinServlet.kt:45)
\n\tat io.javalin.http.JavalinServlet$service$2$1.invoke(JavalinServlet.kt:24)
\n\tat io.javalin.http.JavalinServlet$service$1.invoke(JavalinServlet.kt:129)
\n\tat io.javalin.http.JavalinServlet$service$2.invoke(JavalinServlet.kt:40)\n\tat io.javalin.http.JavalinServlet.service(JavalinServlet.kt:81)
\n\tat javax.servlet.http.HttpServlet.service(HttpServlet.java:790)
\n\tat io.javalin.websocket.JavalinWsServlet.service(JavalinWsServlet.kt:51)
\n\tat javax.servlet.http.HttpServlet.service(HttpServlet.java:790)
\n\tat org.eclipse.jetty.servlet.ServletHolder.handle(ServletHolder.java:763)
\n\tat org.eclipse.jetty.servlet.ServletHandler.doHandle(ServletHandler.java:569)
\n\tat org.eclipse.jetty.server.handler.ScopedHandler.nextHandle(ScopedHandler.java:233)\n\tat org.eclipse.jetty.server.session.SessionHandler.doHandle(SessionHandler.java:1610)\n\tat org.eclipse.jetty.server.handler.ScopedHandler.nextHandle(ScopedHandler.java:233)\n\tat io.javalin.core.JavalinServer$start$wsAndHttpHandler$1.doHandle(JavalinServer.kt:49)\n\tat org.eclipse.jetty.server.handler.ScopedHandler.nextScope(ScopedHandler.java:188)\n\tat org.eclipse.jetty.servlet.ServletHandler.doScope(ServletHandler.java:507)\n\tat org.eclipse.jetty.server.session.SessionHandler.doScope(SessionHandler.java:1580)\n\tat org.eclipse.jetty.server.handler.ScopedHandler.nextScope(ScopedHandler.java:186)\n\tat org.eclipse.jetty.server.handler.ContextHandler.doScope(ContextHandler.java:1292)\n\tat org.eclipse.jetty.server.handler.ScopedHandler.handle(ScopedHandler.java:141)\n\tat org.eclipse.jetty.server.handler.HandlerWrapper.handle(HandlerWrapper.java:127)\n\tat org.eclipse.jetty.server.Server.handle(Server.java:501)\n\tat org.eclipse.jetty.server.HttpChannel.lambda$handle$1(HttpChannel.java:383)\n\tat org.eclipse.jetty.server.HttpChannel.dispatch(HttpChannel.java:556)\n\tat org.eclipse.jetty.server.HttpChannel.handle(HttpChannel.java:375)\n\tat org.eclipse.jetty.server.HttpConnection.onFillable(HttpConnection.java:273)\n\tat org.eclipse.jetty.io.AbstractConnection$ReadCallback.succeeded(AbstractConnection.java:311)\n\tat org.eclipse.jetty.io.FillInterest.fillable(FillInterest.java:105)\n\tat org.eclipse.jetty.io.ChannelEndPoint$1.run(ChannelEndPoint.java:104)\n\tat org.eclipse.jetty.util.thread.strategy.EatWhatYouKill.runTask(EatWhatYouKill.java:336)\n\tat org.eclipse.jetty.util.thread.strategy.EatWhatYouKill.doProduce(EatWhatYouKill.java:313)\n\tat org.eclipse.jetty.util.thread.strategy.EatWhatYouKill.tryProduce(EatWhatYouKill.java:171)\n\tat org.eclipse.jetty.util.thread.strategy.EatWhatYouKill.produce(EatWhatYouKill.java:135)\n\tat org.eclipse.jetty.util.thread.QueuedThreadPool.runJob(QueuedThreadPool.java:806)\n\tat org.eclipse.jetty.util.thread.QueuedThreadPool$Runner.run(QueuedThreadPool.java:938)\n\tat java.base/java.lang.Thread.run(Thread.java:834)\n
@casid
Copy link
Owner

casid commented Oct 30, 2020

Hey @tomholub,

thanks for reporting! I always thought this to be a rather theoretical issue :-) I haven't come close to that limit yet.

In your case, would it work to extract common parts into their own reusable tags? Or is it really a super big file to include?

At work, we have the usecase to include generated css/js files, that are pretty big and don't contain any jte logic at all (they are generated by webpack).

Edit: We use a class like this to load those. Maybe this would be useful to add to jte itself? Would this solve your usecase?

public class JteImportFiles {

   private final Path                          _root;
   private final boolean                       _checkModificationTimes;
   private final ConcurrentMap<String, Long>   _modificationTimes = new ConcurrentHashMap<>();
   private final ConcurrentMap<String, String> _fileContents      = new ConcurrentHashMap<>();

   public JteImportFiles( Path root, boolean checkModificationTimes ) {
      _root = root;
      _checkModificationTimes = checkModificationTimes;
   }

   public String getFileContent( String relativePath ) {
      if ( _checkModificationTimes && hasChanged(relativePath) ) {
         return _fileContents.compute(relativePath, ( k, v ) -> loadFileContentFromDisk(k));
      }

      return _fileContents.computeIfAbsent(relativePath, this::loadFileContentFromDisk);
   }

   private long getLastModified( Path file ) {
      return file.toFile().lastModified();
   }

   private boolean hasChanged( String relativePath ) {
      Long lastResolveTime = _modificationTimes.get(relativePath);
      if ( lastResolveTime == null ) {
         return true;
      } else {
         long lastModified = getLastModified(_root.resolve(relativePath));
         return lastModified != lastResolveTime;
      }
   }

   private String loadFileContentFromDisk( String relativePath ) {
      try {
         Path file = _root.resolve(relativePath);
         _modificationTimes.put(relativePath, getLastModified(file));
         return Files.readString(file, StandardCharsets.UTF_8);
      }
      catch ( IOException e ) {
         throw new UncheckedIOException(e);
      }
   }
}

@tomholub
Copy link
Author

tomholub commented Oct 30, 2020

Definitely - that's what I will be doing (splitting it up). However I didn't expect to run into such limit - you never know what use cases others will run into.

For myself, I wanted to have an all-inclusive "samples" page that has all sorts of html elements or components from our html templates that have not yet been transformed into usable jte components. It's like a repository for our devs where they could go to /samples on a dev build and see all of their choices, find the component that best fits their scenario, and turn that into a jte template.

It's not exactly a production scenario so I'll do something else. But others may be in a different situation.

@casid
Copy link
Owner

casid commented Oct 30, 2020

We're also using this for our styleguide - to display the jte code 'as is' and below the rendered result.

public static Content importFile( String relativePath ) {
   return templateOutput -> templateOutput.writeContent(getFileContent(relativePath));
}

public static String importFileEscaped( String relativePath ) {
   return getFileContent(relativePath);
}

In the styleguide jte template we'd use something like this:

<h2>Code</h2>
${importFileEscaped("jte/tag/styleguide/sample1.jte")}

<h2>Result:</h2>
@tag.styleguide.sample1()

For your usecase you might want to use something like:

<h2>Code</h2>
${importFileEscaped("jte/tag/styleguide/sample1.jte")}

<h2>Result:</h2>
${importFile("jte/tag/styleguide/sample1.jte")}

@casid casid self-assigned this Oct 30, 2020
@casid casid added the enhancement New feature or request label Oct 30, 2020
@tomholub
Copy link
Author

Thanks! That looks good to me. More generally, I could imagine some folks running into this, then silently walking away thinking jte is "not mature enough". So even if there is a way to work around, I'd recommend to see if this can be addressed in the codebase itself 👍

@casid
Copy link
Owner

casid commented Oct 31, 2020

Splitting the generated classes would be pretty hard to do. The entire state of the render() function would have to be passed to the other class. For instance:

!{var x = "foo";}

<%-- Split happens here --%>

${x}

This is stuff the jte compiler isn't aware of, since it simply delegates the generated class to javac.

It would also not help in your example, since there one single String exceeds the limit. One option would be to load all constant Strings from an external location. However, this would slow down template loading, plus, it would prevent the JVM from String inlining. When I did some benchmarks with this, I noticed that write("foo"); is faster than write(CONSTANT). It would be nice if jte supported this, but it would bring some tradeoffs with it in other places. And I haven't come across a situation when it was really needed.

That being said, would it help to add the ability to import file contents in templates to the jte core? Like the JteImportFiles? Or did you end up with an entirely different solution?

@tomholub
Copy link
Author

For myself, I made it a static file and later I'll use importFile as you suggested.

You can leave this issue open for a while first - if others have the same issue, they may leave a note here.

@casid
Copy link
Owner

casid commented Oct 31, 2020

Alright, will do that. Good luck and let me know if you run into any problems with this approach.

@kelunik
Copy link
Collaborator

kelunik commented Oct 31, 2020

@casid This isn't about methods or class files being too long, but rather the constant string size limit of Java, which is 65535 bytes, which is quite low for larger templates that don't contain variables.

I've implemented a fix in #26 and classified this as a bug rather than an enhancement.

@casid casid closed this as completed in #26 Oct 31, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants