Optimize looking for the project root #32

Open
wants to merge 4 commits into
from

Conversation

Projects
None yet
3 participants
@dgutov
Contributor

dgutov commented Oct 31, 2012

When working locally, the process is fast enough, but when using Tramp (over plink), eproject is one of the primary slowdown factors for me.

These changes cut down file opening time from 4-4.5 seconds to 1-1.5, for example, in one "best case" scenario. They don't do much for out-of-project files, though.

dgutov added some commits Oct 31, 2012

Cut down on the selector calls
This way we run all selectors first inside each parent we check,
so we can stop once a selector matches.

Note: It changes the selector API, but so far I haven't seen the FILENAME
argument used anywhere. And in future uses, DIR should be enough for most cases.
Set after-change-major-mode-hook in eproject-maybe-turn-on
This is closer to the original code, but should make no difference for the main
use case: when the buffer's major-mode changes some time after it's opened.

Now the hook will also fire in non-eproject buffers, but I have hard time seeing
the use case. Change the .eproject file, then go to each buffer and flip major
modes?
@jrockway

This comment has been minimized.

Show comment Hide comment
@jrockway

jrockway Dec 7, 2012

Owner

Thanks for this. The change is somewhat complex so I want to test it a bit more before merging, but I haven't forgotten :)

Owner

jrockway commented Dec 7, 2012

Thanks for this. The change is somewhat complex so I want to test it a bit more before merging, but I haven't forgotten :)

@dgutov

This comment has been minimized.

Show comment Hide comment
@dgutov

dgutov Dec 7, 2012

Contributor

Sure, take your time, I mostly gave up on using Tramp anyway, it was slow even with this patch.

One thing to consider is that this change likely directly contradicts the feature request in #36.

Contributor

dgutov commented Dec 7, 2012

Sure, take your time, I mostly gave up on using Tramp anyway, it was slow even with this patch.

One thing to consider is that this change likely directly contradicts the feature request in #36.

@jrockway

This comment has been minimized.

Show comment Hide comment
@jrockway

jrockway Feb 3, 2013

Owner

OK, I think I'm going to incorporate some of these ideas. I will push an update in the next few days and ask for feedback here.

Owner

jrockway commented Feb 3, 2013

OK, I think I'm going to incorporate some of these ideas. I will push an update in the next few days and ask for feedback here.

@stsquad

This comment has been minimized.

Show comment Hide comment
@stsquad

stsquad Apr 10, 2013

Contributor

I think I'm seeing the same problem in my tramp setup. Namely opening a new file under the current project root involves running all the tests again which introduces a lot of latency over a slow tramp link.

Contributor

stsquad commented Apr 10, 2013

I think I'm seeing the same problem in my tramp setup. Namely opening a new file under the current project root involves running all the tests again which introduces a lot of latency over a slow tramp link.

@stsquad

This comment has been minimized.

Show comment Hide comment
@stsquad

stsquad Apr 10, 2013

Contributor

Yeah this shows how slow it can be over tramp:

eproject-maybe-turn-on 1 80.410381247 80.410381247
eproject--run-project-selector 15 80.408251981 5.3605501320
eproject--look-for-impl 15 80.399297201 5.3599531467
eproject--find-file-named 15 80.396797589 5.3597865059
eproject--scan-parents-for 15 80.393950834 5.3595967222
eproject--build-parent-candidates 15 0.0153342080 0.0010222805
eproject--project-selector 15 0.004270832 0.0002847221
eproject--type-info 16 0.0010014669 6.259...e-05
eproject--setup-local-variables 1 0.00087429 0.00087429
eproject--buffer-file-name 16 0.000713578 4.459...e-05
eproject--after-change-major-mode-hook 20 0.0003862789 1.931...e-05
eproject-mode 1 3.6894e-05 3.6894e-05
eproject--project-supertypes 1 2.1522e-05 2.1522e-05
eproject--all-types 1 1.9286e-05 1.9286e-05
eproject-root 3 1.8448e-05 6.149...e-06
eproject-attribute 1 1.5932e-05 1.5932e-05
eproject--init-attributes 1 1.0622e-05 1.0622e-05

Contributor

stsquad commented Apr 10, 2013

Yeah this shows how slow it can be over tramp:

eproject-maybe-turn-on 1 80.410381247 80.410381247
eproject--run-project-selector 15 80.408251981 5.3605501320
eproject--look-for-impl 15 80.399297201 5.3599531467
eproject--find-file-named 15 80.396797589 5.3597865059
eproject--scan-parents-for 15 80.393950834 5.3595967222
eproject--build-parent-candidates 15 0.0153342080 0.0010222805
eproject--project-selector 15 0.004270832 0.0002847221
eproject--type-info 16 0.0010014669 6.259...e-05
eproject--setup-local-variables 1 0.00087429 0.00087429
eproject--buffer-file-name 16 0.000713578 4.459...e-05
eproject--after-change-major-mode-hook 20 0.0003862789 1.931...e-05
eproject-mode 1 3.6894e-05 3.6894e-05
eproject--project-supertypes 1 2.1522e-05 2.1522e-05
eproject--all-types 1 1.9286e-05 1.9286e-05
eproject-root 3 1.8448e-05 6.149...e-06
eproject-attribute 1 1.5932e-05 1.5932e-05
eproject--init-attributes 1 1.0622e-05 1.0622e-05

@dgutov

This comment has been minimized.

Show comment Hide comment
@dgutov

dgutov Apr 10, 2013

Contributor

The optimization in this pull request is about stopping when you found the first project root, it would still perform the scan for each file, because even if a file is inside a project, it may also be inside some subproject, and inner projects have priority. I think we could skip the check when the file is in the same dir as some other file that's already open, though.

The best speedup you can have right now is to just cut down on project definitions. Leave, for example, just one: based on the presence of .git.

Contributor

dgutov commented Apr 10, 2013

The optimization in this pull request is about stopping when you found the first project root, it would still perform the scan for each file, because even if a file is inside a project, it may also be inside some subproject, and inner projects have priority. I think we could skip the check when the file is in the same dir as some other file that's already open, though.

The best speedup you can have right now is to just cut down on project definitions. Leave, for example, just one: based on the presence of .git.

@stsquad

This comment has been minimized.

Show comment Hide comment
@stsquad

stsquad Apr 10, 2013

Contributor

I guess really the optimisation that makes most sense is to check to see if the just opened file meets the criteria for any project currently open first before running the rest of the tests. That way it may take a while on the first file in a tree to be opened but not any ones under it. To be honest the case of subprojects under other projects is not one I deal with and would gladly turn off if I could.

As far as cutting down project definitions is concerned it's a bit of a pain as I use the various definitions to control things like compile definitions and other such things. Sounds like I need to code up a shortcut and see if it works.

Contributor

stsquad commented Apr 10, 2013

I guess really the optimisation that makes most sense is to check to see if the just opened file meets the criteria for any project currently open first before running the rest of the tests. That way it may take a while on the first file in a tree to be opened but not any ones under it. To be honest the case of subprojects under other projects is not one I deal with and would gladly turn off if I could.

As far as cutting down project definitions is concerned it's a bit of a pain as I use the various definitions to control things like compile definitions and other such things. Sounds like I need to code up a shortcut and see if it works.

@stsquad

This comment has been minimized.

Show comment Hide comment
@stsquad

stsquad Apr 11, 2013

Contributor

Well I've submitted #58 as a potential solution to my performance problems, fully acknowledging it breaks sub-projects.

Contributor

stsquad commented Apr 11, 2013

Well I've submitted #58 as a potential solution to my performance problems, fully acknowledging it breaks sub-projects.

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