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

Improve responsiveness of initial project import / show found modules earlier #318

Open
lefou opened this issue Dec 12, 2022 · 8 comments
Open

Comments

@lefou
Copy link

lefou commented Dec 12, 2022

Is your feature request related to a problem? Please describe.

The initial import of my non-trivial Mill project (which is the Mill project itself) is rather slow. The main reason for this exceptional slowness is the fact that the project setup contains integration tests, which trigger a local ivy publish of many of the other project modules.

It seems to me that Metals is working throw the whole spectrum of BSP requests to get a full picture of my project. Unfortunately, any outcome, e.g. a list of modules, is only presented to the user at the end of that process.

Technically, the local publish, which triggers a full packaging of other modules, only needs to happen when the integration tests are actually executed. We expose the location of the local ivy repository via a environment variable, so we trigger the build in the <integrationtest>.forkEnv target, which is triggered by the BSP jvmRunEnvironment request.

Describe the solution you'd like

As the workspaceBuildTargets request is send and completed very early, it would be very nice, if Metals could reflect the project modules before it sends further BSP requests. This might cause some incomplete module status, but from a user perspective, early feedback is a huge win.

Describe alternatives you've considered

I don't think the user has any alternatives to waiting.

Additional context

No response

Search terms

import, Mill, responsiveness

@lefou
Copy link
Author

lefou commented Dec 12, 2022

After some more discussions in the discord channel, I think the core of this request is to hold requests for runtime settings back until the editor is successfully configured. Preparing runtime settings isn't of any use until the user is able to edit the project. In my concrete setup, this would avoid to run those time consuming test environment preparations before the editor is usable.

@lefou
Copy link
Author

lefou commented Dec 12, 2022

I update the initial description, as the initial trigger was the jvmRunEnvironemnt request (not the buildTargetScalaMainClasses).

@tgodzik
Copy link
Contributor

tgodzik commented Dec 12, 2022

I update the initial description, as the initial trigger was the jvmRunEnvironemnt request (not the buildTargetScalaMainClasses).

Thanks! We should be able to postpone that after indexing.

@lefou
Copy link
Author

lefou commented Feb 19, 2024

This is still an issue will current Metals and the Mill repository. An initial import takes minutes, although the buildTargets request returned after some seconds, returning 199 modules. It would be really great, if the UI and the Metals Doctor could reflect this, so users know and can inspect why they're waiting.

@tgodzik
Copy link
Contributor

tgodzik commented Feb 20, 2024

The reason might be the same as with Bazel, we need to ask for Scalac options, but that comes with the classpath, so the recent changes to BSP I did should help out here I hope.

@lefou
Copy link
Author

lefou commented Feb 20, 2024

The reason might be the same as with Bazel, we need to ask for Scalac options, but that comes with the classpath, so the recent changes to BSP I did should help out here I hope.

Ok, I probably can't expect you to change to whole internals of Metals, so this seems fair. By "recent changes to BSP" you probably mean the following PR?

It's still waiting for approval, I guess. Do you plan to support the upcomming BSP 2.2.0 before it gets final? I think a final 2.2.0 may take some time. There is some tooling update pending (move to Bazel). Also, the website lists only BSP 2.1.0, so it's a bit tough right now to design for the new changes.

@tgodzik
Copy link
Contributor

tgodzik commented Feb 20, 2024

By "recent changes to BSP" you probably mean the following PR?

Yep, that's what I mean.

It's still waiting for approval, I guess. Do you plan to support the upcomming BSP 2.2.0 before it gets final? I think a final 2.2.0 may take some time. There is some tooling update pending (move to Bazel). Also, the website lists only BSP 2.1.0, so it's a bit tough right now to design for the new changes.

I will need to think of something, maybe it would make sense to backport it to 2.1.x branch?

@lefou
Copy link
Author

lefou commented Feb 20, 2024

I will need to think of something, maybe it would make sense to backport it to 2.1.x branch?

I don't think that would be wise. It effectively is an addition to the protocol, which should end up in a minor version bump. Although, the BSP project could just release it as 2.2.0 and postpone the current PRs to a 2.3.x.

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