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

Racer spawns a large number of one-off threads... why? #157

Closed
renato-zannon opened this issue Feb 18, 2015 · 2 comments
Closed

Racer spawns a large number of one-off threads... why? #157

renato-zannon opened this issue Feb 18, 2015 · 2 comments

Comments

@renato-zannon
Copy link
Contributor

On src/racer/ast.rs, there are multiple methods whose implementation looks something like this:

    let thread = Thread::scoped(move || {
        let stmt = string_to_stmt(s);
        let mut v = ImplVisitor { name_path: None, trait_path: None };
        visit::walk_stmt(&mut v, &*stmt);
        return v;
    });
    return thread.join().ok().unwrap();

Since this is using Thread::scoped (which makes the parent thread wait on the thread.join() call), there isn't any gain in performance here - on the contrary, there's a big performance loss. On my linux system, a call to racer complete std::old_io:: spawns 108 threads, and spends almost half of its execution time on the futex syscall (thread synchronization).

On a comment, there's a possible justification for this - handling panic!s from the parser, since Rust provides no way to "catch" panic!s. The problem with this justification is twofold - first, methods as the one above (which are by far the most common on the file I mentioned) aren't actually catching anything, since they unwrap() the final result of thread.join() - meaning that a panic! will be propagated to the parent thread anyway. Secondly, per rust-lang/rust#22435, "catching" panics at the border of a scoped thread won't be supported by the stdlib anymore - which means that racer won't be able to use this technique anymore in a few days.

I'm opening this issue so that we can discuss a possible solution for this - hopefully one that doesn't use threads for control flow :)

@phildawes
Copy link
Collaborator

Hi Renato,
You are correct in thinking it was originally to catch panics. Racer has got much better at passing parsable blobs of code to libsyntax now so it's not needed and remains mainly because of my lazy copy-paste coding style! Feel free to fix if you want, or I'll repair it when rust-lang/rust#22435 hits.

BTW Thanks for pointing me at rust-lang/rust#22435 - I'll check out the discussion.

Cheers,

Phil

@phildawes
Copy link
Collaborator

This is great, thanks Renato

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