[todo] refactor out auto module detection handlings
The discussion has continued from https://github.com/JunoLab/Atom.jl/pull/192#issuecomment-536489355.
In short, we now have 3 alternatives to detect a file's module:
- old way based on CodeTools: https://github.com/JunoLab/Atom.jl/blob/9cf10173e4c5727451e43950872d6cdb3795c720/src/eval.jl#L13-L39
- Revise-like approach: https://github.com/JunoLab/Atom.jl/blob/9cf10173e4c5727451e43950872d6cdb3795c720/src/utils.jl#L175-L359
- CSTParser-based approach: https://github.com/JunoLab/Atom.jl/blob/9cf10173e4c5727451e43950872d6cdb3795c720/src/goto.jl#L185-L205
1 is being used for module handler, 2 and 3 are being used for the recent goto (and renamerefactor, in the future) handler.
The main purpose of this refactor is to implement a more robust module detection logic in the module handler by replacing 1. with something like a combination of 2. and 3.
Or at least, since 1. and 3. are kinda duplicated works, so we want to unify the logic. (and this will hopefully leads to less JIT compiles)
We may need to significantly refactor the frontend code as well -- it's old too.
@pfitzseb can you bundle the related issues here (link to an issue report if exists or just list the random confirmed cases), so that we can easily check if we could address them by this refactor ?
P.S. Feel free to edit this issue for the future reference. We want more structured information for this.
https://github.com/JunoLab/Atom.jl/pull/210 serves as a preparation for this.
A little interesting fact on this: both modulefiles(mod::Module) and CSTParser-based module traverse runs in almost the same amount of time:
I changed these lines into
const profile_id = Ref{Int}(0)
function _collecttoplevelitems(mod::Module)
profile_id[] += 1
return if profile_id[] % 2 === 0
# Revise-like approach
entrypath, paths = modulefiles(mod)
__collecttoplevelitems(stripdotprefixes(string(mod)), [entrypath; paths])
else # CSTParser-based approach
entrypath, line = moduledefinition(mod)
__collecttoplevelitems(stripdotprefixes(string(mod)), entrypath)
end
end
and precompiled Atom module (to make sure modulefile works for Atom module), and then profiled globalgotoitems like below
Juno.@profiler begin
for i = 1:30
Atom.globalgotoitems("SYMBOLSCACHE", Atom, nothing, "")
Atom.clearsymbols()
end
end
, which would cause both Revise-like module file finding and CSTParser-based module traverse in turn.
And the profiling yielded that almost most same amount of time were spent on entrypath, paths = modulefiles(mod) and __collecttoplevelitems(stripdotprefixes(string(mod)), entrypath) (CSTParser-based module traverse), which means both ways work differently but there is no much difference between their performances.
As for goto, obviously CSTParser-based module traverse is better since we have to call toplevelitems anyway even if Revise-like file revealing succeeded. But in reality they are only called once for each module, so this won't be a big deal anyway.