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

Use incremental mode (and daemon mode, once implemented) #14

Closed
FichteFoll opened this issue Oct 11, 2017 · 7 comments
Closed

Use incremental mode (and daemon mode, once implemented) #14

FichteFoll opened this issue Oct 11, 2017 · 7 comments

Comments

@FichteFoll
Copy link
Collaborator

Upstream issue: python/mypy#3968

Hopefully this will speed up linting enough that we can enable checks for unsaved files.

@gerardroche
Copy link
Contributor

Just to let you know, I've been using incremental mode without any issues. At least none that I can tell.

@FichteFoll
Copy link
Collaborator Author

You mean with tmpfile_suffix = "py" instead of "-"? When I implemented it, mypy was way too slow to lint my project and it took about 3s to show lint results, which is way too long for "real-time linting".

What version of mypy are you running and how fast is it for you?

gerardroche added a commit to gerardroche/SublimeLinter-contrib-mypy that referenced this issue Jan 29, 2018
@gerardroche
Copy link
Contributor

Maybe I think I'm using incremental mode when actually fact I'm not.

I just manually added --incremental to the cmd options like this:


    def cmd(self):
        """Return a list with the command line to execute."""
        cmd = [
            self.executable,
            '*',
            '--follow-imports=silent',  # or 'skip'
            # '--ignore-missing-imports',
            '--show-column-numbers',
            '--hide-error-context',
            '--incremental',
            '@'
        ]

My mpy version is 0.560.

If I uncomment the the tempfile_suffix = '-' then I get errors trying find modules:

/tmp/tmpre1sal:8:9: Cannot find module name" 

There were some issues with the regex matching for tmp file errors which I just fixed in #18, along with several other regex matching issues. So now when you uncomment tempfile_suffix = '-' and add --incremental to the cmd options mypy will work except it can't find modules.

@gerardroche
Copy link
Contributor

gerardroche commented Jan 29, 2018

I've tested it, and yes incremental is working for me, and it makes a huge difference. I intially made a mistake: I had incremental enabled in my project mypy.ini configuration file, but I was trying to manually enable and disabling it in the plugin and couldn't understand why I couldn't see a performance difference... there wasn't one, because incremental mode was always enabled.

The runtime difference for my project is maybe ~2/3s vs 0.4/0.5s.

Without incremental mode everything needs to be generated for each run. You can see the impact it has by enabling verbose mode in the plugin:

        cmd = [
            self.executable,
            '*',
            '--follow-imports=silent',  # or 'skip'
            # '--ignore-missing-imports',
            '--show-column-numbers',
            '--hide-error-context',
            '--incremental',
            # '--quick-and-dirty',
            '--verbose',
            '@'
        ]

There are still some issues.

Enabling it for unsaved files (commenting tempfile_suffix = '-') breaks imports (notice above that I disable --ignore-missing-imports), so you get errors like this:

SublimeLinter: mypy output:
    /tmp/tmpy2ff_6:5:0: error: Cannot find module named 'Name.a.b'
    /tmp/tmpy2ff_6:5:0: note: (Perhaps setting MYPYPATH or using the "--ignore-missing-imports" flag would help)

You can fix those errors by always using the --shadow-file option:

So instead of:


        if self.tempfile_suffix == "-":
            # --shadow-file SOURCE_FILE SHADOW_FILE
            #
            # '@' needs to be the shadow file,
            # while we request the normal filename
            # to be checked in its normal environment.
            # Trying to be smart about view.is_dirty and optimizing '--shadow-file' away
            # doesn't work well with SublimeLinter internals.
            cmd[-1:] = ['--shadow-file', self.filename, '@', self.filename]

Always use this instead:

cmd[-1:] = ['--shadow-file', self.filename, '@', self.filename]

But I don't really know what shadow file does. It works, I just don't don't really know why.

Mypy is is still slow-ish for realtime linting. My CPU spins a little when mypy runs, so when I have it enabled for unsaved files it's constantly whirring.

@FichteFoll
Copy link
Collaborator Author

I've looked into the mypy code to understand what shadow file actually does (took me a while to figure out), but it works as advertised.

Thanks for looking into the --incremental option. For some reason I thought this was either already active or not yet implemented so I didn't bother checking further. This will certainly improve run-time behavior immensely. I'll make that change later today, and look at your PR.

@FichteFoll
Copy link
Collaborator Author

FichteFoll commented Jan 30, 2018

Btw, the conditional for adding --shadow-file was reversed. It should have tested for tempfile_suffix to be anything other than "-".

FichteFoll added a commit that referenced this issue Jan 30, 2018
I have no idea why this isn't active by default since I don't see why
mypy would need to create a cache dir (`.mypy_cache`) otherwise,
but specifying this arg lowers runtime by about 50% for the
SublimeLinter codebase, bringing it to about 1s on my machine.

This actually makes it usable as a live linter using the --shadow-file
arg, so we tell SL to do that.

Closes #14.
@gerardroche
Copy link
Contributor

👍 works like a charm. It seems even faster now in some situations, like when the file hasn't changed. Also, I'm now using the --quick-and-dirty option withou any issues. You can configure that from your project mypy.ini file.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

2 participants