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

Improving @require performance #39

Closed
rofinn opened this issue Dec 2, 2017 · 6 comments
Closed

Improving @require performance #39

rofinn opened this issue Dec 2, 2017 · 6 comments

Comments

@rofinn
Copy link

rofinn commented Dec 2, 2017

I'm not sure if there's much that can be done, but the @require appears be hurting my load times by an order of magnitude. I'm guessing this has something to do with what code is able to be cached between julia sessions?

Loading Glob.jl and URIParser.jl w/ the appropriate glue code:

julia> @time using FilePaths
  0.067642 seconds (37.85 k allocations: 2.538 MiB)

Using 2 @require blocks for the Glob.jl and URIParser.jl glue code (not loading those packages):

julia> @time using FilePaths
  0.527255 seconds (367.49 k allocations: 20.297 MiB, 14.84% gc time)

NOTES: I have precompile enabled and this is after the cache has been updated between changes. I also ran it a few times for each and got fairly consistent results.

@timholy
Copy link
Member

timholy commented Dec 3, 2017

Likely similar to timholy/Revise.jl#50.

@rofinn
Copy link
Author

rofinn commented Dec 4, 2017

Thanks, much better with just 1 @schedule:

julia> @time using FilePaths
  0.090361 seconds (40.75 k allocations: 2.667 MiB)

The downside is that it introduces race conditions.

@rofinn
Copy link
Author

rofinn commented Dec 4, 2017

Alright, I think part of my issue is still just that my glue code can't get precompiled with the rest of my package. I tried just optionally evaluating the code in my __init__ and that performs well when I don't need the glue code, but it's very slow when I do.

@MikeInnes
Copy link
Collaborator

Can I take a look at how you're doing this? Couldn't see a branch on your repo.

@rofinn
Copy link
Author

rofinn commented Dec 4, 2017

My branch is kind of a mess right now, but in my __init__ in FilePaths.jl I'm doing:

function __init__()
    @sync begin
        @async begin
            if Base.isbindingresolved(Main, :Glob)
                Glob = getfield(Main, :Glob)

                @eval begin
                    """
                        glob{T<:AbstractPath}(path::T, pattern::AbstractString) -> Array{T}

                    Returns all subpaths along the provided `path` which match the glob `pattern`.

                    # Example
                    ```
                    julia> glob(p"src", "*.jl")
                    9-element Array{FilePaths.PosixPath,1}:
                    p"src/FilePaths.jl"
                    p"src/constants.jl"
                    p"src/deprecates.jl"
                    p"src/libc.jl"
                    p"src/mode.jl"
                    p"src/path.jl"
                    p"src/posix.jl"
                    p"src/status.jl"
                    p"src/windows.jl"
                    ```
                    """
                    function $(Glob).glob{T<:AbstractPath}(path::T, pattern::AbstractString)
                        matches = $(Glob).glob(pattern, String(path))
                        map(T, matches)
                    end
                end
            end
        end

        @async begin
            if Base.isbindingresolved(Main, :URIParser)
                URIParser = getfield(Main, :URIParser)

                @eval begin
                    """
                        uri(path::AbstractPath) -> AbstractPath

                    Creates a `file://` `URI` from the `path`.
                    """
                    function uri(path::AbstractPath)
                        if isempty(root(path))
                            error("$path is not an absolute path")
                        end

                        uri_str = "file://$(String(path))"

                        return $(URIParser).URI(uri_str)
                    end
                end
            end
        end
    end
end

I was previously just wrapping those 2 function definitions (glob and uri) in @require begin ... end blocks.

@timholy
Copy link
Member

timholy commented Dec 13, 2021

There's been a lot of performance work done on this package.

@timholy timholy closed this as completed Dec 13, 2021
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

3 participants