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

Incompatibilities with package precompilation #43

Closed
MichaelHatherly opened this issue Dec 20, 2024 · 5 comments
Closed

Incompatibilities with package precompilation #43

MichaelHatherly opened this issue Dec 20, 2024 · 5 comments

Comments

@MichaelHatherly
Copy link

MichaelHatherly commented Dec 20, 2024

Thank you for writing and maintaining this package @MommaWatasu.

Due to the usage of eval in the code below

return eval(Expr(:->,
Expr(:tuple, map(Symbol, collect(keys(init)))...),
body))
template objects can't be called during package precompilation since they evaluate into an already closed module. Doing so would break incremental compilation. See the stacktrace below:

julia> import MWE
Precompiling MWE
        Info Given MWE was explicitly requested, output will be shown live
ERROR: LoadError: Evaluation into the closed module `OteraEngine` breaks incremental compilation because the side effects will
not be permanent. This is likely due to some other module mutating `OteraEngine` with `eval` during precompilation - don't do t
his.
Stacktrace:
  [1] eval
    @ ./boot.jl:385 [inlined]
  [2] eval
    @ ~/.julia/packages/OteraEngine/rkuaf/src/OteraEngine.jl:1 [inlined]
  [3] build_render(elements::Vector{Union{AbstractString, OteraEngine.JLCodeBlock, OteraEngine.SuperBlock, OteraEngine.TmpBlock
, OteraEngine.TmpCodeBlock, OteraEngine.VariableBlock}}, init::Dict{String, String}, newline::String, autoescape::Bool)
    @ OteraEngine ~/.julia/packages/OteraEngine/rkuaf/src/template.jl:171
  [4] (::OteraEngine.Template)(; init::Dict{String, String})
    @ OteraEngine ~/.julia/packages/OteraEngine/rkuaf/src/template.jl:108
  [5] Template
    @ ~/.julia/packages/OteraEngine/rkuaf/src/template.jl:99 [inlined]
  [6] test
    @ ~/personal/MWE/src/MWE.jl:13 [inlined]
  [7] macro expansion
    @ ~/personal/MWE/src/MWE.jl:18 [inlined]
  [8] macro expansion
    @ ~/.julia/packages/PrecompileTools/L8A3n/src/workloads.jl:78 [inlined]
  [9] macro expansion
    @ ~/personal/MWE/src/MWE.jl:17 [inlined]
 [10] macro expansion
    @ ~/.julia/packages/PrecompileTools/L8A3n/src/workloads.jl:140 [inlined]
 [11] top-level scope
    @ ~/personal/MWE/src/MWE.jl:16
 [12] include
    @ ./Base.jl:495 [inlined]
 [13] include_package_for_output(pkg::Base.PkgId, input::String, depot_path::Vector{String}, dl_load_path::Vector{String}, load
_path::Vector{String}, concrete_deps::Vector{Pair{Base.PkgId, UInt128}}, source::Nothing)
    @ Base ./loading.jl:2285
 [14] top-level scope
    @ stdin:3
in expression starting at /Users/mike/personal/MWE/src/MWE.jl:1
in expression starting at stdin:3
  ✗ MWE
  0 dependencies successfully precompiled in 7 seconds. 3 already precompiled.

ERROR: The following 1 direct dependency failed to precompile:

The definition of MWE is

module MWE

using OteraEngine
using PrecompileTools

function template()
    txt = "Hello {{ usr }}!"
    return Template(txt, path=false)
end

function test()
    tmp = template()
    return tmp(; init=Dict("usr" => "OteraEngine"))
end

@setup_workload begin
    @compile_workload begin
        test()
    end
end

end # module MWE

with the following versions used:

(MWE) pkg> st
Project MWE v0.1.0
Status `~/personal/MWE/Project.toml`
  [b2d7f28f] OteraEngine v0.6.1
  [aea7be01] PrecompileTools v1.2.1

julia> VERSION
v"1.10.7"

If the precompilation workload is removed then the following example can be run and is illustrative of the problem:

julia> import MWE

julia> import OteraEngine

julia> tmp = MWE.template();

julia> length(names(OteraEngine; all = true))
94

julia> tmp(; init=Dict("usr" => "OteraEngine"))
"Hello OteraEngine!"

julia> length(names(OteraEngine; all = true))
95

julia> tmp(; init=Dict("usr" => "OteraEngine"))
"Hello OteraEngine!"

julia> length(names(OteraEngine; all = true))
96

Each time a defined template is called an entirely new function object is defined within the OteraEngine module.

Ideally the fix for this is evaluating just once per template into the calling module, rather than on each render into OteraEngine itself. Whether that's a doable change to the package implementation isn't something I can tell.

@MommaWatasu
Copy link
Owner

Hi @MichaelHatherly

I've tried (so hard) to eliminate eval function form my package but I found it difficult.
The first reason is custom filter and julia code block. They are defined in user space and impossible to use it without eval as far as I know.
And the second reason is that even for or if statements are hard to implement without eval. I know I can implement them using generated function and that would make the rendering faster. But the form of these statements vary and I'm not sure whether it's possible to cover all the cases. For example, how should I implement following two statements in generated function? Unfortunately I don't know...

# case 1
for i in 1 : 10
# case 2
for i in numbers

What's worse, you can't know how many elseif statements succeed after if statement, which makes it harder to implement in generated function.

I also really want to implement my package using generated function. If you have any idea to do this, please let me know...

@MichaelHatherly
Copy link
Author

Passing around the calling Module as part of the Template object would likely work. You'd need some way for the Template constructor to either explicitly take the Module, or switch to using a @Template macro which could then just know what it is via __module__. You'd then need to ensure any evals that get called are passed the Module and also best to switch to Core.eval(mod, ex) instead so that it doesn't use the OteraEngine.eval definition.

I'd honestly try avoiding generated functions, they've got limitations around what you should be putting in them. Just using a macro-based approach to get the code generated at the right time should be viable for this kind of thing. Obviously it's all up to you in the end though :)

@MichaelHatherly
Copy link
Author

The last part that I mentioned

Each time a defined template is called an entirely new function object is defined within the OteraEngine module.

is kind of separate to the issue of which module to eval into. It's a memory leak in long-running server processes if OteraEngine is used to render templates, since every request will result in a new Function object getting created. Those are GC'd by Julia so the server will eventually run out of memory. (You can see this behaviour if you just call a single template in a while true loop and watch the process start eating up memory.

@MommaWatasu
Copy link
Owner

Passing around the calling Module as part of the Template object would likely work. You'd need some way for the Template constructor to either explicitly take the Module, or switch to using a @Template macro which could then just know what it is via __module__. You'd then need to ensure any evals that get called are passed the Module and also best to switch to Core.eval(mod, ex) instead so that it doesn't use the OteraEngine.eval definition.

I got it. That was exactly what I was looking for. I'm not sure how long does it take to apply this idea, but I'll try.

@MommaWatasu
Copy link
Owner

I changed the implementation of Template (especially build_renderer function) a lot to enable Precompilation and avoid taking up memory every time a Template is rendered. Now this module works correctly:

The definition of MWE is

module MWE

using OteraEngine
using PrecompileTools

function template()
    txt = "Hello {{ usr }}!"
    return Template(txt, path=false)
end

function test()
    tmp = template()
    return tmp(; init=Dict("usr" => "OteraEngine"))
end

@setup_workload begin
    @compile_workload begin
        test()
    end
end

end # module MWE

I'll release this change as v1.0.0 (because, with this change, we no longer waste memory even when rendering multiple times on a server, etc.), so try it after it's released.
Please re-open this issue if you notice any bug.
Thanks again for your report and suggestions!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants