search

julia-vscode / SymbolServer.jl

Other
23 stars 31 forks source link

improved name resolution #185

Closed pfitzseb closed 4 years ago

pfitzseb commented 4 years ago

The newly added all_names has a few advantages over split_module_names:

  1. Seems to be about an order (or two) of magnitude faster: 
    
julia> @time all_names(JSON);
0.000248 seconds (51 allocations: 7.281 KiB)

julia> @time SymbolServer.split_module_names(JSON, allns); 0.015986 seconds (19.93 k allocations: 1.255 MiB)

2. It seems to be more correct (in this specific case its `Base.parse` being shadowed by `JSON.Parsers.parse`, which is imported into `JSON` with `using .Parsers: parse`):

julia> :parse in all_names(JSON) true

julia> a, b = SymbolServer.split_module_names(JSON, allns);

julia> :parse in a || :parse in b false



This is only a very superficial integration of that function, mostly because I'm not 100% confident I haven't missed anything and would like to hear @ZacLN's opinion on this approach first :)

@timholy might also be interested in this, given his recent work on performance.
ZacLN commented 4 years ago

Side note - could we all try to keep formatting changes to separate PRs?

pfitzseb commented 4 years ago

Yeah, sorry about that -- seems like VSCode is autoformatting files on save :)

aviatesk commented 4 years ago

We turn on auto-formatting on save here, but we may want to make that TS-specific.

pfitzseb commented 4 years ago

Eh, I like that behaviour. It's just that the kwarg option is different between what I have locally set and this codebase. So either we fix that in .vscode or reformat our Julia code.