Skip to content

Decouple summarizer and code generator #678

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

Closed
mahesh-hegde opened this issue Mar 22, 2023 · 1 comment
Closed

Decouple summarizer and code generator #678

mahesh-hegde opened this issue Mar 22, 2023 · 1 comment

Comments

@mahesh-hegde
Copy link
Contributor

mahesh-hegde commented Mar 22, 2023

Motivation:

  • The source based summarizer is slow, especially on low end machine. This makes jnigen end-to-end tests take much more time than that of ffigen.

  • We should be able to test the summary generation easily, and also be able to add tests which ensure both parsers generate similar information.

  • Ideally, we should be able to either cache the summary, or pass the same summary to different tests. Test bindings using both Dart-only and Dart+C modes #685

  • Ideally, adding a new parser should be easier in future. For example, to parse kotlin sources, or to parse incomplete java sources.

Design 1: caching summary

  • summary is cached in .dart_tool/jnigen/summary-{config_hash}.
  • It's invalidated when any .java file in source_path or .jar or .class file in class_path is newer than the cached summary, or related config is modified.
  • When there are more than, say, 5 summary caches, least recently used one is evicted, (or upon a hash collision).

Design 2: Explicitly dumping and passing summary

This is a simpler design.

  • jnigen:write_summary file --config=... writes summary to specified file

  • jnigen ..... --summary-file= reads the summary from specified file instead of parsing.

Other tasks

  • Ensure overload-renaming order is same with any parser. This can be achived by some sorting in proper place.
@mahesh-hegde mahesh-hegde mentioned this issue Mar 22, 2023
6 tasks
@mahesh-hegde
Copy link
Contributor Author

I think as @HosseinYousefi mentioned elsewhere, it's better to test each part of the pipeline separately. Separating getSummary() part in code is trivial and dart-archive/jnigen#220 already does that to enable some tests. So closing this as not planned.

@mahesh-hegde mahesh-hegde closed this as not planned Won't fix, can't repro, duplicate, stale Apr 2, 2023
@HosseinYousefi HosseinYousefi transferred this issue from dart-archive/jnigen Nov 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

No branches or pull requests

1 participant