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

Further frontend performance optimisations #234

Closed
wants to merge 14 commits into from

Conversation

mlange05
Copy link
Collaborator

@mlange05 mlange05 commented Mar 2, 2024

Another small round of tricks to improve the performance of the sanitize_ir utility and the FP frontend to reduce the frontend processing time for large input subroutines. Primarily, this PR focuses on using fast in-place transformers for the necessary IR sanitisation and avoid using the explicit rescoping for the FParser frontend by attaching the scopes on first IR node creation.

In more detail:

  • Use an on-the-fly `Transformer to do inline-comment attachment
  • Make all transformers used during sanitisation use inplace=True
  • Avoid calling rescope_symbols on the finalised IR when creating subroutines

On the pathological test case we the following improvements:

# On main
In [2]: with config_override({'log-level': 'perf'}):
   ...:     Sourcefile.from_file("apl_arpege_parallel.F90")
[Loki::Frontend] Executed sanitize_input in 0.12s
[Loki::FP] Executed parse_fparser_source in 7.41s
[Loki::Frontend] Executed sanitize_ir in 0.25s
[Loki::Frontend] Executed sanitize_ir in 5.70s
[Loki::Frontend] Executed sanitize_ir in 0.94s
[Loki::FP] Executed parse_fparser_ast in 19.30s
[Loki::Sourcefile] Constructed from apl_arpege_parallel.F90 in 26.83s

# with this PR
In [2]: with config_override({'log-level': 'perf'}):
   ...:     Sourcefile.from_file("apl_arpege_parallel.F90")
[Loki::Frontend] Executed sanitize_input in 0.12s
[Loki::FP] Executed parse_fparser_source in 7.76s
[Loki::Frontend] Executed sanitize_ir in 0.09s
[Loki::Frontend] Executed sanitize_ir in 2.13s
[Loki::Frontend] Executed sanitize_ir in 0.47s
[Loki::FP] Executed parse_fparser_ast in 9.32s
[Loki::Sourcefile] Constructed from apl_arpege_parallel.F90 in 17.27s

Integration note: PR is currently passing ECphysics regression, but not local test base. Will attempt to fix local tests ASAP, but filing draft now to trigger test base. Initial bulk-parse in EC-physics also seems to improve with this.

@mlange05 mlange05 requested a review from reuterbal March 2, 2024 11:02
Copy link

github-actions bot commented Mar 2, 2024

Documentation for this branch can be viewed at https://sites.ecmwf.int/docs/loki/234/index.html

@mlange05 mlange05 force-pushed the naml-frontend-improvements-part-deux branch from dadfafb to e75fedb Compare March 7, 2024 16:04
As variable declarations can be encoutered out of sequence, we have
rebuild `DeferredTypeSymbols` after parsing the full spec, so that
the symbols might be rebuild correctly.

This commit adjusts a test to capture this and adds a small custom
mapper to do this in place.
@mlange05 mlange05 force-pushed the naml-frontend-improvements-part-deux branch from e75fedb to c534e4c Compare March 8, 2024 13:30
@reuterbal
Copy link
Collaborator

With #241, #242 and #245 merged, can this be closed?

@mlange05
Copy link
Collaborator Author

Yes, indeed this had been done via separate PRs.

@mlange05 mlange05 closed this Mar 12, 2024
@mlange05 mlange05 deleted the naml-frontend-improvements-part-deux branch March 12, 2024 18:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants