Upgrade HtmlFlow to release 3.5 #56
Conversation
* Fixed a problem where the block html was written two times into the response. Reported by @Vest at jreijn#34 (comment) * Updated the HtmlFlowIndexView according to the new behavior of HtmlFlow in version 3.3. (https://github.com/xmlet/HtmlFlow#33-august-2019) where it:”disallows the use of chained calls to dynamic() due to unexpected cache behaviors.”. Thus we had to replace the inner invocations of dynamic() by of(). * Changed the static method getView() of HtmlFlowIndexView with a static field view. * Added the threadSafe() initialization to the previous view to enable safe use on concurrent benchmarks.
|
@fmcarvalho I didn't have time to review your new PR. I found out that you replaced the public function getView with the static variable view. So it means that comparing to other engines, that we were using in this repository, your view is not recreated on every test iteration. I debugged your code for quite a long time and I found out that with this change, the shared DynamicHtml object uses HtmlVisitorCache, where the property isCached is equal to true for every thread that is created by Spring. E.g. if you put the breakpoint here: // HtmlFlowView.renderMergedTemplateModel
String html = HtmlFlowIndexView.view.render(model);and step into // DynamicHtml.render
public final String render(T model) {
binder.accept(this, model);
return getVisitor().finished(); // <-- here
}you will find that the method finished reads cached parts of the dynamic content from cacheBlockList. See the image below:
Maybe I am wrong, but I guess that what happens is unfair to other engines, where the view is built and parsed at runtime for each request individually. In your case, we just take strings from the cache. I will return my change back, but leave your PR with the new version of HtmlFlow. @jreijn if you are interested in this finding, you can test the PR as well. |
|
@Vest You are not right when you said: "the method finished reads cached parts of the dynamic content from cacheBlockList.". It only reads the static parts and not the dynamic parts. As you can see in your print screen you only have 2 blocks in You can observe it by passing different models to this view and you will see different outputs. The same approach happens with textual templates engines. The static parts are only concatenated with the dynamic parts. For example in JSP we do: Yet, in HtmlFlow since the static parts are defined through functions, we must first invoke those functions to get the output of the static part, that is in turn stored in the Maybe the name |
|
yes, it seems that I was wrong. I apologize. This name was really confusing, because I was trying to understand what kind of data is cached and why by the time when the rendering starts, the string builder is initialized with the data. Ok, it seems fine, or at least I didn't find anything suspicious. Thanks |
|
@Vest We are already using the term static in some code of this class such as BTW when I am running the benchmarks I am getting slightly different results from those ones published in benchmarks-102019. I know those results depend of many different environment characteristics and hardware. But, I cannot observer JSP and Freemaker being the top two most performant. And I cannot observe either the JSP gaining for almost 3 or 4 seconds above competence. |
|
I can repeat the tests tomorrow just for your curiosity, but indeed these results are not 100% precise. Sometimes they are 73% reproducible :) |
|
@Vest did you have the opportunity to run the tests again? Did you see differences in results? btw I forgot to update the htmlflow version in README.md to 3.5. |
HtmlFlowIndexViewaccording to the new behavior of HtmlFlow in version 3.3. (https://github.com/xmlet/HtmlFlow#33-august-2019) where it: ”disallows the use of chained calls todynamic()due to unexpected cache behaviors.”. Thus we had to replace the inner invocations ofdynamic()byof().getView()ofHtmlFlowIndexViewwith a static fieldview.threadSafe()initialization to the previous view to enable safe use on concurrent benchmarks.