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

--native-lib #7450

Open
ncannasse opened this issue Sep 21, 2018 · 14 comments
Open

--native-lib #7450

ncannasse opened this issue Sep 21, 2018 · 14 comments
Assignees
Milestone

Comments

@ncannasse
Copy link
Member

While we're changing the commandline implementation, I think we should review our definition of "native library".

ATM we have:

  • haxelib path can return -L for native neko libs. There's no other way to inject them
  • Flash have -swf-lib, C# has -net-lib , Java has -java-lib

I propose regrouping all of the there under a single --native-lib (with aliases to older -swf-lib etc.), and have a single com.native_libs.

@ncannasse ncannasse added this to the Release 4.0 milestone Sep 21, 2018
@RealyUniqueName
Copy link
Member

Let's make --native-lib work for other targets as well. That would be consistent.
E.g. it can behave like Compiler.includeFile() for lua and js.
Python and php could import native files.
Maybe dll/so for C++ if it's possible.

@back2dos
Copy link
Member

What does haxe.macro.Compiler.addNativeLib do right now though? ^^

@ncannasse
Copy link
Member Author

ncannasse commented Sep 22, 2018 via email

@Simn Simn modified the milestones: Release 4.0, Release 4.1 Dec 1, 2018
@Simn Simn modified the milestones: Release 4.1, Release 4.0 Aug 10, 2019
@Simn Simn self-assigned this Aug 10, 2019
@Simn
Copy link
Member

Simn commented Aug 10, 2019

I think we have to do something here for 4.0. I'm not just talking about --native-lib as a unified flag, but also about the native library vs. compilation server situation.

Related issues: #7417 and #7651

Our native library handling was never updated to support the compilation server properly. On every request, which includes display requests, the functionality kicks in and does something.

The way it currently works is this:

  1. Open and parse the native library file
  2. Set up a lookup to resolve dot-paths to parsed modules (in com.load_extern_type)

It's important to note that step 2 is lazy: The native representation is not converted to a Haxe module unless explicitly requested via dot-path.

The problem is that we perform step 1 on every request. Even worse, C# actually goes on a hunt in the file system to find its net-std file, which is part of the problem in #7651.


I propose the following approach when the compilation server is running:

  • For each context, keep track of which native library files have been processed. Also track the timestamp so we can detect file changes.
  • When processing --native-lib, check that cache. If the file has been processed, do nothing.
  • If it has not, perform the native -> Haxe conversion eagerly for all types. This is a bit expensive, but we only have to do it once per context. Because we're only storing the parsed representation, this should not lead to a huge memory usage increase.

This would greatly simplify the handling on the compilation server because native libraries become a one-time read-and-forget thing. It will automatically fix related display issues like #7417 because we deal with the syntax representation as if we had normal Haxe modules.

We can still keep load_extern_type around for the offline mode because there lazy loading certainly makes sense.

Any thoughts?

@Aurel300
Copy link
Member

What if the eager loading finds some types in a library that cause an error? With lazy loading you can simply avoid importing/using these types.

@Simn
Copy link
Member

Simn commented Aug 10, 2019

Technically, that shouldn't happen in the first place. But putting a catch-all around this might be a good idea regardless.

@Simn
Copy link
Member

Simn commented Aug 12, 2019

My approach doesn't work because the parser-cache is file-based. This would require a separate caching mechanism, which makes matters much more complicated.

@ncannasse
Copy link
Member Author

ncannasse commented Aug 14, 2019 via email

@Simn
Copy link
Member

Simn commented Aug 14, 2019

I'm not quite sure what you're saying we should cache. There are 4 stages when it comes to processing a native library:

  1. Read the data from the filesystem
  2. Convert it to the native representation
  3. Convert that to Haxe AST
  4. Type the Haxe AST

I'm proposing to cache 3. because that's what the display handling needs and I doubt there's much of a size difference between 2. and 3.

Simn added a commit that referenced this issue Aug 16, 2019
@Simn
Copy link
Member

Simn commented Aug 16, 2019

Regarding the actual new CLI argument, there's the need to configure some things per-library (e.g. a flag to ignore unknown types in #3397, or whether to include a native library in the output).

How about we allow JSON input for this new flag? It would be --native-lib '{ "file": "lib/whatever.jar", "ignoreUnknownTypes": true, "includeInOutput": false }'. We can still accept --native-lib lib/whatever.jar for simplicity.

Simn added a commit that referenced this issue Aug 16, 2019
* [cli] add --native-lib and --native-lib-extern

closes #3469
see #7450
closes #8080

* two-pass native library handling

* don't write to stderr because that fucks up the comp server

* handle extern libs differently

Both Java and C# need access to all libs in java_libs/net_libs in order to implement their `lookup` functions.

* hold back on --native-lib CLI argument for now
@RealyUniqueName
Copy link
Member

Having to type json in CLI args makes me sad.
Can't we have --native-lib-file, --native-lib-ignore-unknown-type etc? Or -D flags?

@Simn
Copy link
Member

Simn commented Aug 17, 2019

How would that work in practice given that the setting is per-library?

@back2dos
Copy link
Member

What's the relationship between the compiler arg and Compiler.addNativeLib? Could we rely on adding optional args to the latter to handle the more complex use cases?

If that's not an option, I would suggest that the --native-lib may in fact be a JSON file, that contains the config (including relative path to the actual .jar/.dll (should probably default to a sensible convention)). So instead of --native-lib '{ "file": "lib/whatever.jar", "ignoreUnknownTypes": true, "includeInOutput": false }' one would write --native-lib lib/whatever.json with that file containing { "ignoreUnknownTypes": true, "includeInOutput": false }.


On a side note: I wonder why we're using JSON for everything lately. For some of this config stuff, I think haxe expressions might be a better fit. Not only are they nicer to write (less quotes, allow comments, trailing commas, could even contain enums where it makes sense), we also have a position tracking parser for them. And I suspect the typer could be leveraged to guarantee validity (would probably need a separate context to not cause mayhem - perhaps there's a way to eagerly populate it with types and then disable further type loading it it?) and give things like unknown field ingoreUnknownTypes, (Suggestion: ignoreUnknownTypes) with exact positions.

@Simn
Copy link
Member

Simn commented Aug 17, 2019

I like the idea to allow --native-lib lib/whatever.json. That keeps the CLI clean and still allows to convey all the required information.

@Simn Simn added this to the Release 4.2 milestone Feb 19, 2020
@RealyUniqueName RealyUniqueName modified the milestones: Release 4.2, Backlog Dec 14, 2020
@Simn Simn modified the milestones: Backlog, Later Mar 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants