Enhancement: When parent template is initialized with a PrintStream, any internal use of addPartial should use implicitly that PrintStream so we avoid having to pass the PrintStream through the model parameter

[nightswimmings]

DynamicHtml with a given PrintStream uses PrintStringBuilder internally, which buffers the content.
This prevents a major benefit of using a custom PrintStream, which is outputting big amounts of content without getting a JavaHeap exception. With the advent of PWA, having hundreds of Mb on the html is not surprising.

It would be nice if we got at least an option to not buffer content. I almost managed to hack it myself with a custom HtmlView class, but tuning the logic on HtmlVisitorCache.visitOpenDynamic requires going too deep into the intrincates of the library, since it expects the substring() call to return buffered content

[nightswimmings]

We might also simply make PrintStringBuilder efficient so it clears StringBuilder buffer ondemand after content is not needed. My particular case fails because I add a very long stream of Stream.forEachOrdered(model -> view.addPartial(DynamicHtml.view.... which keeps storing all rendered partials into the StringBuffer instead of printing them and skip the buffering

[fmcarvalho]

@nightswimmings Can you please share your use case in a github repo that we could check?

[nightswimmings]

This is my exact code (removing all noise and changing the model to Penguin)

DynamicHtml.view(writer, this::template).write(Stream.of(penguins));


private void template(DynamicHtml<Stream<Penguin>> view, Stream<Penguin> model) {
    try {
        view.html()
                .head()
                    .title().text("MyPenguinExample").__()
                .__() //head
                .body()
                    .div().attrClass("local-storage").attrStyle("display: none;")
                        .dynamic(localStorageDiv -> model.forEachOrdered(penguin -> view.addPartial(DynamicHtml.view(this::penguinPartialTemplate), penguin)))
                    .__()
                .__() //body
              .__(); //html
    } catch (IOException e){
        throw new RuntimeException("Error building HTML template", e);
    }
}

private void penguinPartialTemplate(DynamicHtml<Penguin> view, Penguin penguin){
    view.div().attrId("data-penguin-" + penguin.getName()).attrStyle("display: none;").addAttr("data-dna",penguin.getDnaCode());
}

If the Penguins stream is very long, and/or the penguin.getDnaCode() contains a lot of data, when all the streamed partials get buffered internally, sooner or later, a JavaHeap Exception occurs (even though the PrintStream is printing to the output stream as well in real time) . The example works flawlessly with a moderate amount of data

[nightswimmings]

Forget about it... I found the culprit... it is just that the PrintStream needs to be passed as part of the model and used here: view.addPartial(DynamicHtml.view(WRITER,...). I am really sorry for wasting your time.

Anyway, when PrintStream is passed on the parent DynamicHtml.view(writer, this::template).write(Stream.of(penguins)) call, internally all the addPartials should use it implicitly, but this is a much different issue

[fmcarvalho]

Thank you @nightswimmings Give me time to read all this carefully. I don't have time now. But I would like to analyze your use case in detail. It is always useful not only to enhance this library but also to enrich our documentation with real usage scenarios.

[nightswimmings]

Yes, I guess my sample is really interesting in the sense that it mixes the logic on the second example of documentation section "Dynamic Views" (which shows how to template a foreach scenario) with the one on the "Partial And Layout" (which shows how to embed a subtemplate/partial)

[fmcarvalho]

Sorry for my late follow up. I have been overwhelmed. I tried your use case and I am not observing that behavior. In truth, for me it behaves exactly as you describe in the title of this Issue and HtmlFlow already supports the desired behavior.

I added a new unit test regarding your example that you may find on development branch here: https://github.com/xmlet/HtmlFlow/blob/development/src/test/java/htmlflow/test/TestPartialsToPrintStream.java

In line 66 I am omitting the WRITER and it will use the parent output PrintStream. Ate least, the Assertion succeeded with all the expected output.

Looking for the source it also seems to implement the desired behavior on addPartial as:

public final <U> void addPartial(HtmlView<U> partial, U model) {
        getVisitor().closeBeginTag();
        partial.getVisitor().depth = getVisitor().depth;
        if (this.getVisitor().isWriting())
            getVisitor().write(partial.render(model));
    }

In getVisitor().write(partial.render(model)); it is requesting the parent view Visitor that contains the output PrintStream and writes the partial content gathered from partial.render(...).

I will try with a very long stream to check if I can observe the initial Issue regarding a JavaHeap exception. But using .addPartial within a .dynamic() should avoid any king of buffering.

BTW, could it be the case of your were using an .of() rather than a .dynamic?

[fmcarvalho]

Ok, I have simulated the Issue with large data. I am looking for the problem and a solution now.

The problem is not related with your suggested guess of using the parent Visitor that is already being used. The issue is really in your first report of the internal behavior of the PrintStringBuilder.

[fmcarvalho]

There are a couple of constraints related with backwards compatibility. According to HtmlFlow design a view has a visitor stored in a final field. In turn, this visitor determines the output approach: 1) to a PrintStream or 2) returning a resulting String. And this decision is made on instantiation through the factory methods DynamicHtml.view(...) or StaticHtml.view(...).

Partial views have no difference from any other view and that is a nice principle.

The tricky part on above statement getVisitor().write(partial.render(model)) is that when you instantiate the partial with a WRITER the render returns null and the output has been already flushed on render() call.

On the other hand, creating the partial without an output PrintStream makes render() always buffering into an internal StringBuilder

[fmcarvalho]

@nightswimmings I have already released version 3.7 of HtmlFlow that solves this issue. Please check it if you can.

It provides the feature reported in this issue, that means when parent template is initialized with a PrintStream, any internal use of addPartial should use implicitly that PrintStream to avoid having to pass the PrintStream through the model parameter.

Also I have tested in a large data scenario in a new test class TestPartialsToPrintStream in unit test testAddPartialWithLargeData: https://github.com/xmlet/HtmlFlow/blob/master/src/test/java/htmlflow/test/TestPartialsToPrintStream.java#L62 and it works fine.

The new implementation of addPartial resorts on the following snippet and the enhancement is on partial.clone(v.newbie()).render(model):

public final <U> void addPartial(HtmlView<U> partial, U model) {
        getVisitor().closeBeginTag();
        partial.getVisitor().depth = getVisitor().depth;
        if (this.getVisitor().isWriting()) {
            HtmlVisitorCache v = getVisitor();
            String p = partial.clone(v.newbie()).render(model);
            if(p != null) v.write(p);
        }
    }

Once the visitor is stored in a final field, we must clone the partial view. Since our views and templates are data-structure less avoiding in memory trees and nodes, and the HTML document is based on a higher-order function, then our views instances only store a few instance fields related to the visitor and the template function itself. Thus we do not expect overhead from the clone() and newbie().