a little compile for generating cached module parsers #1816

jurgenvinju · 2023-06-22T14:39:34Z

for every module file M.rsc that requires a concrete syntax parser, this PR checks if M.parsers is right there next to it. If so then it uses ParseTree::loadParsers instead of generating a parser (and loading a parser generator!).
to generate a bunch of .parsers file, this PR contains lang::rascal::grammars::storage::ModuleParserStorage which provides void storeParsersForModules(PathConfig pcfg). This will produce the required .parser file in the bin parameter location of the pathConfig, based on the relative file path of the rsc file in the srcs parameter of the pathConfig.
so to use this:
- call storeParsersForModules once, i.e. during mvn package or just before
- make sure the .rsc files that are loaded from the jar are copied right next to the .parsers file
- wait and see
success is visible in the absence of log messages that are "loading the parser generator" modules. Also the first module loads 1000x faster and subsequent modules 20 to 30 times faster than before.
please give it a spin. I'm curious if this is enough to solve Prevent "bootstrapping" a full parser generator for deployed DSL implementations rascal-language-servers#267

codecov · 2023-06-22T14:45:50Z

Codecov Report

Merging #1816 (b6c0b93) into main (329130b) will increase coverage by 0%.
The diff coverage is 68%.

@@           Coverage Diff           @@
##              main   #1816   +/-   ##
=======================================
  Coverage       48%     48%           
- Complexity    6045    6062   +17     
=======================================
  Files          670     670           
  Lines        58660   58668    +8     
  Branches      8546    8547    +1     
=======================================
+ Hits         28658   28674   +16     
+ Misses       27835   27823   -12     
- Partials      2167    2171    +4

Impacted Files	Coverage Δ
src/org/rascalmpl/uri/URIUtil.java	`42% <62%> (+3%)`	⬆️
src/org/rascalmpl/semantics/dynamic/Import.java	`55% <72%> (-1%)`	⬇️
...almpl/interpreter/result/SourceLocationResult.java	`50% <100%> (-1%)`	⬇️

... and 8 files with indirect coverage changes

…e syntax modules faster

…ModuleFile.rsc instance that has concrete syntax in it

DavyLandman

The code in general looks good, I have some questions about the function in URIUtil.

DavyLandman · 2023-06-22T18:48:18Z

src/org/rascalmpl/uri/URIUtil.java

@@ -433,4 +433,32 @@ public static ISourceLocation removeOffset(ISourceLocation prev) {
    public static ISourceLocation createFromURI(String value) throws URISyntaxException {
        return vf.sourceLocation(createFromEncoded(value));
    }
+    public static ISourceLocation changeExtension(ISourceLocation location, String ext) throws URISyntaxException {
+        String path = location.getPath();
+		boolean endsWithSlash = path.endsWith(URIUtil.URI_PATH_SEPARATOR);


nitpick: strange indent? mixed tab/spaces?

it's a clone from SourceLocationResult; I'll see what I can do about that.

DavyLandman · 2023-06-22T18:49:25Z

src/org/rascalmpl/uri/URIUtil.java

+        String path = location.getPath();
+		boolean endsWithSlash = path.endsWith(URIUtil.URI_PATH_SEPARATOR);
+		if (endsWithSlash) {
+			path = path.substring(0, path.length() - 1);


I'm a bit confused why changing the extension of an entry that ends with a path seperator would then suddenly contiue doing a string operation on directory name? What's the semantics of that?

We have a semantics for "filenames" that includes directory names. A filename is the last segment of the path list. If a URI ends with a / we interpret the characters between that slash and the previous as the "filename", because that is the name of a directory in that case. Filenames can have extensions, and so for the sake of consistency, directory names can also have extensions. From there on you get to write this kind of code to make it all consistent.

All of this was discovered some years ago while writing simple random tests for the field update feature of Rascal. If you update a file name that does not have an extension, and then ask back for the extension, (or the new filename), then you want to see the extension back (or the filename with the extension) and not "hello/.txt" or the ".txt" for the new filename, or even "", just because the URI accidentally was a directory.

DavyLandman · 2023-06-22T18:51:50Z

src/org/rascalmpl/uri/URIUtil.java

+
+		if (path.length() > 1) {
+			int slashIndex = path.lastIndexOf(URIUtil.URI_PATH_SEPARATOR);
+			int index = path.substring(slashIndex).lastIndexOf('.');


lastIndexOf takes an second argument int fromIndex, that would make the downstream code (and this as well) a bit simpler.

I'm leaving this as-is. That code has worked for ages and is used daily. All I did was factor it out of the Result hierarchy. Later it will move to ISourceLocation implementations in vallang. Then we can have another look at optimality IMHO.

okay, I reviewed this as new code, since I didn't see a subsequent removal of the same code from somewhere else. I didn't realise it's a clone, could we add some comments to help someone that is fixing a bug do it at the two places where this code exists? Or otherwise rewrite the result side to use this function?

I think I can factor this out of SourceLocationResult now. Then there is only one copy. Next we'll move it to ISourceLocation implementations.

The nasty part of this is that URIUtil's change methods do not carry over offset, lengths and such. We have to fix that when we move them to ISourceLocation.

DavyLandman · 2023-06-22T18:53:19Z

src/org/rascalmpl/uri/URIUtil.java

+				path = path + (!ext.startsWith(".") ? "." : "") + ext;
+			}
+			else if (!ext.isEmpty()) {
+				path = path.substring(0, slashIndex + index) + (!ext.startsWith(".") ? "." : "") + ext;


There are 4 different cases handled in this function, where for my intuition, only one of them matches the name of changeExtension. I think this function either needs a "broader" name, or some javadoc in front of it to explain what it does.

sure I'll document it. It is the exact semantics of field updates in locations i n Rascal.

The update with the empty string is extension removal indeed,

if an extension was present it is replaced

if an extension was not present the new one is added

and that times the difference between not-ending or ending with a slash. So there are 6 cases I believe.

o, and then there is the two cases of not having a "." and having a "." in the replacement part. So that makes 12 cases.

DavyLandman · 2023-06-22T18:54:31Z

src/org/rascalmpl/library/lang/rascal/grammar/storage/ModuleParserStorage.rsc

+This caching feature is _static_. There is no automated cache clearance.
+If your grammars change, any saved `.parsers` files do not change with it. 
+It is advised that you programmatically execute this compiler at deployment time
+to store the `.parsers` file _only_ in deployed `jar` files. That way, you can not


agreed :) maybe we should print a warning if the src path is the same as the binpath?

I don't see a big issue with that. Only you'd have to use a resource copier to get everything into the jar. it would help with testing the feature if you print next to the source files, since the current interpreter would pick them up immediately.

ah, I might have thought too quickly that we could detect this happening.

DavyLandman · 2023-06-22T19:03:34Z

I'll give this a spin next week, looks good.

Is this something we could add to the rascal-maven plugin?

jurgenvinju · 2023-06-22T19:45:01Z

I'll give this a spin next week, looks good.

Is this something we could add to the rascal-maven plugin?

yes we could do that
or you could call the function directly from the generate-sourced maven-plugin already, using some simple maven configuration kung-fu.

I'll try 2. tomorrow and see if we can include that XML in the docs. If it is too hard, I'll write another MOJO.

jurgenvinju · 2023-06-23T10:59:23Z

I merged this and making a 0.32.0-RC1 release of Rascal. This is required to be able to run the tutor on the new standard library, and also helpful for experimenting with rascal-language-servers.

scaffolding a little compiler for generating cached parsers

278ac1e

jurgenvinju mentioned this pull request Jun 22, 2023

Prevent "bootstrapping" a full parser generator for deployed DSL implementations usethesource/rascal-language-servers#267

Closed

jurgenvinju added 3 commits June 22, 2023 19:39

wrote mini compiler for generating cached parsers for parsing concret…

9d39d8c

…e syntax modules faster

added docs

8bc2aad

instrumented interpreter to reuse ModuleFile.parsers files for every …

9ddbda8

…ModuleFile.rsc instance that has concrete syntax in it

jurgenvinju marked this pull request as ready for review June 22, 2023 18:02

jurgenvinju requested a review from DavyLandman June 22, 2023 18:02

DavyLandman approved these changes Jun 22, 2023

View reviewed changes

jurgenvinju added 2 commits June 23, 2023 09:22

removed clone of extension replacement in URI

5c06b14

added docs on how and when to call the ModuleParserStorage

b6c0b93

jurgenvinju merged commit cb35d4b into main Jun 23, 2023
7 checks passed

jurgenvinju deleted the rascal-module-parser-storage branch June 23, 2023 10:57

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

a little compile for generating cached module parsers #1816

a little compile for generating cached module parsers #1816

jurgenvinju commented Jun 22, 2023 •

edited

Loading

codecov bot commented Jun 22, 2023 •

edited

Loading

DavyLandman left a comment

DavyLandman Jun 22, 2023

jurgenvinju Jun 22, 2023

DavyLandman Jun 22, 2023

jurgenvinju Jun 22, 2023

DavyLandman Jun 22, 2023

jurgenvinju Jun 22, 2023

DavyLandman Jun 23, 2023

jurgenvinju Jun 23, 2023

jurgenvinju Jun 23, 2023

DavyLandman Jun 22, 2023

jurgenvinju Jun 22, 2023

jurgenvinju Jun 22, 2023

DavyLandman Jun 22, 2023

jurgenvinju Jun 22, 2023

DavyLandman Jun 23, 2023

DavyLandman commented Jun 22, 2023

jurgenvinju commented Jun 22, 2023

jurgenvinju commented Jun 23, 2023

a little compile for generating cached module parsers #1816

a little compile for generating cached module parsers #1816

Conversation

jurgenvinju commented Jun 22, 2023 • edited Loading

codecov bot commented Jun 22, 2023 • edited Loading

Codecov Report

DavyLandman left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DavyLandman commented Jun 22, 2023

jurgenvinju commented Jun 22, 2023

jurgenvinju commented Jun 23, 2023

jurgenvinju commented Jun 22, 2023 •

edited

Loading

codecov bot commented Jun 22, 2023 •

edited

Loading