When interacting with a document, prepare the target it belongs to #1307
Conversation
|
@swift-ci Please test |
| if configuredTargets.count <= targets.count { | ||
| preparationTasksToAwait.append(task) | ||
| } | ||
| fallthrough | ||
| case nil: | ||
| targetsToPrepare.append(target) |
There was a problem hiding this comment.
I might be misunderstanding, but won't this mean that we'd still schedule the preparation again for the same target if we already have one running?
There was a problem hiding this comment.
Yes, it does and that’s intentional, in the context of multi-target preparation. Say that while background indexing you are preparing targets A, B, and C in a single SwiftPM invocation (like a hypothetical swift prepare --target A --target B --target C), which has the benefit that it allows concurrent building of the three targets if they don’t depend on each other.
Now you are trying to work on a file in A, which requires target A to be prepared. Now, you don’t want to wait for preparation of all three targets to finish, but want to build target A as quickly as possible. So, you schedule preparation of A with high priority, which will cancel and reschedule the preparation of A, B and C in TaskScheduler. Once preparation of A has finished, the joint preparation of A, B, and C is re-scheduled, which will effectively only need to prepare B and C because the build system realizes that A is already up-to-date.
Does that make sense?
There was a problem hiding this comment.
Yeah that makes sense. I was mainly thinking of the case where you have A being prepared and get a new request to prepare A, we'll schedule it again. Though I wasn't really thinking about the scheduling logic, now that I think about it, we'll make the subsequent request wait on the former, and then when evaluating it should be a no-op, so I guess this isn't the end of the world.
There was a problem hiding this comment.
This comment doesn't quite seem true though:
// target's preparation status with a longer-running task. The key benefit here is that when we get many
// preparation requests for the same target (eg. one for every text document request sent to a file), we don't
// re-create new `PreparationTaskDescription`s for every preparation request.
There was a problem hiding this comment.
Ah, I was just missing a break here.
…hat are known to be up-to-date Fixes swiftlang#1258 rdar://127475948
Whenever we get request for a document, open it or edit it, trigger a preparation of its target, but don’t block any interaction based on it. This should ensure that the target is usually prepared when the user is interacting with it. We need to track the preparation status of targets somewhat accurately in `SemanticIndexManager`, so we don’t unnecessarily re-prepare a target. When updating the index store, it is acceptable to schedule another `UpdateIndexStoreTaskDescription` because it will early exit based on an `mtime` check with the unit file. Null builds of a target take significantly longer and thus we want to avoid them. Fixes swiftlang#1252 rdar://127474003
ahoppen
force-pushed
the
background-prepare-targets
branch
from
6f65659 to
7208376
Compare
ahoppen
changed the title
|
@swift-ci Please test |
|
@swift-ci Please test Windows |
Whenever we get request for a document, open it or edit it, trigger a preparation of its target, but don’t block any interaction based on it. This should ensure that the target is usually prepared when the user is interacting with it.
We need to track the preparation status of targets somewhat accurately in
SemanticIndexManager, so we don’t unnecessarily re-prepare a target. When updating the index store, it is acceptable to schedule anotherUpdateIndexStoreTaskDescriptionbecause it will early exit based on anmtimecheck with the unit file. Null builds of a target take significantly longer and thus we want to avoid them.Fixes #1252
Fixes #1258
rdar://127474003
rdar://127475948
rdar://127473987