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

add resolve only pass #243

Merged
merged 4 commits into from
Jan 22, 2021
Merged

add resolve only pass #243

merged 4 commits into from
Jan 22, 2021

Conversation

ZacLN
Copy link
Contributor

@ZacLN ZacLN commented Jan 20, 2021

Adds a new option for semantic_pass whereby only 'target' expressions get the full treatment (i.e. build new scopes, add bindings etc) for the rest we only resolve references. The logic isn't fully worked out but the idea is that we'd only run everything on the toplevel scope and those parts of the tree (delayed scope - e.g. functions) that are new - for the rest we only need to ensure that any new bindings that have been added to the toplevel scope are resolved.

@ZacLN
Copy link
Contributor Author

ZacLN commented Jan 21, 2021

Some timings:

using CSTParser, StaticLint, SymbolServer, BenchmarkTools
server = StaticLint.FileServer()
ssi = SymbolServerInstance("/your/depot/path", "/your/cache/path")
_, server.symbolserver = SymbolServer.getstore(ssi, "~/.julia/environments/v1.7/")
server.symbol_extends  = SymbolServer.collect_extended_methods(server.symbolserver)


function setup(rootp, fp, server)
    empty!(server.files)
    root = StaticLint.loadfile(server, rootp)
    StaticLint.semantic_pass(root)
    f = StaticLint.getfile(server, fp)
    root, f
end

begin
    root, f = @btime setup(Base.find_source_file("Base.jl"), Base.find_source_file("abstractarray.jl"), server);
    @btime (StaticLint.clear_meta.(CSTParser.EXPR[f.cst[32]]);StaticLint.semantic_pass(root))
    @btime (StaticLint.clear_meta.(CSTParser.EXPR[f.cst[32]]);StaticLint.semantic_pass(root, CSTParser.EXPR[f.cst[32]]))
    @btime StaticLint.semantic_pass(f)
end
# 723.366 ms (3663143 allocations: 229.04 MiB)
#  171.288 ms (253585 allocations: 15.83 MiB)
#  86.797 ms (81905 allocations: 4.32 MiB)
#  4.115 ms (5311 allocations: 395.45 KiB)

So we get about a 2x speedup on handling edits. This can probably be sped up further if we feed in information on what has actually changed at the toplevel (i.e. with this approach we're still trying to resolve anything that hasn't got a reference - we could limit that by only trying to resolve symbols matching new names that have been added).

@ZacLN
Copy link
Contributor Author

ZacLN commented Jan 21, 2021

Subject to julia-vscode/CSTParser.jl#224 being merged, and a new verison tagged, this should be good to go.

@ZacLN ZacLN marked this pull request as ready for review January 21, 2021 14:25
davidanthoff
davidanthoff previously approved these changes Jan 22, 2021
Copy link
Member

@davidanthoff davidanthoff left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't understand enough to really review this, but superficially it all looks great :) So, I'd say we merge.

Does this depend on a new CSTParser version? Shouldn't some test fail in that case?

@ZacLN ZacLN merged commit 916f076 into master Jan 22, 2021
@ZacLN ZacLN deleted the incremental-update branch January 22, 2021 21:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants