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

chore: deduplicate GritQL snippets #4153

Merged
merged 5 commits into from
Oct 7, 2024

Conversation

arendjr
Copy link
Contributor

@arendjr arendjr commented Oct 1, 2024

Summary

When matching GritQL snippets against a syntax tree, it tries various contexts in which the syntax could occur. These contexts allow it to parse snippets that would not be valid syntax if they were pasted into a top-level source file. Unfortunately, some of them could still end up parsing to the same syntax tree, in which case we end up matching redundant trees. By deduplicating, we can speed up the snippet matching.

Test Plan

Let's see what Codspeed says :)

@github-actions github-actions bot added the L-Grit Language: GritQL label Oct 1, 2024
@arendjr arendjr requested review from a team October 1, 2024 16:39
@arendjr arendjr changed the title perf(gritql): deduplicate GritQL snippets perf(grit): deduplicate GritQL snippets Oct 1, 2024
Copy link

codspeed-hq bot commented Oct 1, 2024

CodSpeed Performance Report

Merging #4153 will improve performances by 7.37%

Comparing arendjr:deduplicate-snippets (35aa0b4) with main (2f0d5c7)

Summary

⚡ 1 improvements
✅ 104 untouched benchmarks

Benchmarks breakdown

Benchmark main arendjr:deduplicate-snippets Change
db_17847247775464589309.json[cached] 13.8 ms 12.9 ms +7.37%

@arendjr arendjr marked this pull request as draft October 2, 2024 07:14
@arendjr arendjr changed the title perf(grit): deduplicate GritQL snippets chore: deduplicate GritQL snippets Oct 7, 2024
@arendjr arendjr marked this pull request as ready for review October 7, 2024 17:33
@arendjr
Copy link
Contributor Author

arendjr commented Oct 7, 2024

Unfortunately, this PR doesn't show any performance improvements in Codspeed. It could very well be that any improvements are specific to particular queries. This would make sense, since not all queries are parseable in multiple source contexts. In any case, it seems deduplicating is a good thing to do, so I'll merge anyway and maybe it gives some improvements in some situations. Oh well 🤷

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
L-Grit Language: GritQL
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant