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

feat(#3490): Attempt To Speed Up The add-refs.xsl #3491

Merged
merged 9 commits into from
Nov 15, 2024

Conversation

volodya-lombrozo
Copy link
Member

@volodya-lombrozo volodya-lombrozo commented Nov 12, 2024

In this PR I change add-refs.xsl logic a bit:

  1. Removed the usage of axis (like `ancestor).
  2. Added the usage of indexes <xsl:key name="o-by-name" match="o[@name]" use="@name"/>

This potentially might speed up the add-refs.xsl transformation on particular files. However, initial performance measurements show rather contradictory results.

  1. For example, the benchmark in eo shows performance degradation:
# Before
add-refs.xsl                         15331  5.98%
add-refs.xsl                         30528   5.96%
add-refs.xsl                         45057   5.76%

# After
add-refs.xsl                         62579   6.14%
add-refs.xsl                         84454   6.48%
add-refs.xsl                         84454   6.48%
  1. However, on real files I can see significant improvement in some scenarios:
# Function.xmir 
# Before
14:12:38.018 [main] WARN  -- XSL 'add-refs' took 7s (over 5ms)
14:12:58.628 [main] WARN -- XSL 'add-refs' took 21s (over 5ms)
14:15:11.617 [main] WARN -- XSL 'add-refs' took 5s (over 5ms)
14:15:25.214 [main] WARN -- XSL 'add-refs' took 14s (over 5ms)
14:16:27.251 [main] WARN -- XSL 'add-refs' took 6s (over 5ms)
14:16:42.055 [main] WARN -- XSL 'add-refs' took 15s (over 5ms)
# After
14:58:03.620 [main] WARN -- XSL 'add-refs' took 909ms (over 5ms)
14:58:18.763 [main] WARN -- XSL 'add-refs' took 15s (over 5ms)
14:59:26.707 [main] WARN -- XSL 'add-refs' took 800ms (over 5ms)
14:59:44.261 [main] WARN -- XSL 'add-refs' took 18s (over 5ms)
15:00:51.300 [main] WARN -- XSL 'add-refs' took 807ms (over 5ms)
15:01:07.589 [main] WARN -- XSL 'add-refs' took 16s (over 5ms)

and

# Native.xmir
# Before
15:20:10.931 [main] WARN -- XSL 'add-refs' took 20s (over 5ms)
15:22:25.565 [main] WARN -- XSL 'add-refs' took 2min (over 5ms)
15:22:27.988 [main] WARN -- XSL 'add-refs' took 452ms (over 5ms)
# After
15:10:03.688 [main] WARN -- XSL 'add-refs' took 1s (over 5ms)
15:12:25.355 [main] WARN -- XSL 'add-refs' took 2min (over 5ms)
15:12:27.995 [main] WARN -- XSL 'add-refs' took 510ms (over 5ms)

Relate to #3490.


PR-Codex overview

This PR focuses on enhancing the check-xsl-id.groovy file by updating the nameFilter to include xs3p.xsl, while also improving the documentation in README.md and adding new functionality to the add-refs.xsl file with new functions for node processing.

Detailed summary

  • Updated nameFilter in check-xsl-id.groovy to include xs3p.xsl.
  • Corrected typos in README.md.
  • Added xmlns:eo and xmlns:xs attributes to the xsl:stylesheet in add-refs.xsl.
  • Introduced new functions: eo:closest, eo:has-intersecting-route, and eo:get-route for enhanced node processing.

✨ Ask PR-Codex anything about this PR by commenting with /codex {your question}

@volodya-lombrozo
Copy link
Member Author

@yegor256 Could you take a look, please?

@yegor256
Copy link
Member

@volodya-lombrozo I'm sorry, but we had to make some changes in the master, in order to fix a bug. Please, merge it into your branch (resolving one conflict).

@yegor256 yegor256 merged commit a0a4afe into objectionary:master Nov 15, 2024
19 checks passed
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