Generate virtual documents for namespace functions without source refs #251
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
lionel-
commented
Feb 26, 2024
This was referenced Feb 26, 2024
|
First anecdotal note - starting up R is now much slower. i.e. opening dplyr now leaves me in the I am somewhat certain that our r-idle-async tasks end up just running immediately and block because of this recursive case check, since Screen.Recording.2024-02-27.at.10.21.30.AM.mov |
|
Running > devtools::load_all()
ℹ Loading dplyr
Warning message:
Problem while running user `onLoad` hook for package dplyr.
ℹ The hook inherits from `package:base`.
Caused by error in `fun()`:
! argument "path" is missing, with no default Ah this seems to be because pkgload doesn't supply the 2nd |
|
Unexpected longjmp error I've never seen before. I was typing this It seems to truly be related to the Full traceback below, but here are some related bits It seems to be hitting reproducibly when just typing in Update, ok for some unknown reason we actually do hit |
Interesting, I noticed a slowdown too but much smaller.
oh that makes sense, I'll fix that by never running immediately in the case of idle tasks |
crates/ark/src/srcref.rs
Outdated
| // FIXME: Move this to Backend? | ||
| pub static mut ARK_VDOCS: Lazy<DashMap<String, Vec<String>>> = Lazy::new(|| DashMap::new()); | ||
|
|
||
| pub static ARK_VDOC_REQUEST: &'static str = "ark/internal/getVirtualDocument"; |
There was a problem hiding this comment.
The naming of these typically doesn't have get in it, i.e. /statementRange or /helpTopic. Do you think it could just be /vDoc or /virtualDocument? Just trying to be consistent.
Similarly the method could just be vdoc() or virtual_document(), which also matches how the statement range / help topic naming is
There was a problem hiding this comment.
This is now "ark/internal/virtualDocument"
lionel-
force-pushed
the
feature/fake-source
branch
from
b67690c
to
d845cdd
Compare
lionel-
force-pushed
the
feature/fake-source
branch
from
8b9f454
to
cb0bcaf
Compare
lionel-
force-pushed
the
feature/fake-source
branch
2 times, most recently
from
83a21ae
to
cc5cbaa
Compare
|
@DavisVaughan Ready for review! |
lionel-
commented
Jun 7, 2024
lionel-
commented
Jun 7, 2024
lionel-
force-pushed
the
feature/fake-source
branch
from
ef023ec
to
e055cd0
Compare
lionel-
force-pushed
the
feature/fake-source
branch
from
e055cd0
to
f56252e
Compare
This was referenced Jun 11, 2024
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Addresses posit-dev/positron#2285
Addresses part of posit-dev/positron#1729
Addresses part of posit-dev/positron#29
Closes posit-dev/positron#2284
Requires posit-dev/positron#2320
When source references for a namespace are missing, we now deparse the functions into a virtual document. On the frontend side this document is accessible via the
ark:URI scheme. Since we don't know the original file organisation of the package, the whole namespace is stored in a single source file, e.g.ark:namespace:base.Rfor base.This is all that is needed to support stepping with the debugger (posit-dev/positron#1729). Supporting jump-to-definition and find-usages will require more work.
Supportive new features implemented here:
onLoadevent. R only supportsonLoadhooks for specific namespaces known in advance. We extend the hook system to generate an event in Ark for the loading of any namespace.assign()method that is used to update the namespaces in placeThe process goes as follows:
Idle tasks are registered to populate a virtual source for the namespace for:
In a namespace, look for all functions missing source refs. Deparse the functions to text and reparse them to an AST with source references pointing to the virtual document. Supported by the
srcfileargument foparse()and line directives#line nadded to the deparsed source.If the AST does not correspond exactly, discard it and ignore this function. This happens when the function has been generated programmatically or when there is a bug in the deparser, e.g.
foo$'bar'is deparsed asfoo$barand so the roundtrip doesn't correspond exactly.In these cases debugger stepping will be supported by a fallback path (Implement debugging fallback when there are no srcrefs #249)
If it corresponds, update the function in place. We use a bit of a hacky trick to reuse the bytecode of the original function because recompiling is much too slow.