Skip to content

Conversation

@rafaelfranca
Copy link
Collaborator

This make easier to change the filter logic.

I'm working on reorganizing this project to allow multiple benchmarks per libraries in the same folder, but this code isn't easy to change, do I'm working my way down the code to make my change and future ones easier.

@rafaelfranca rafaelfranca requested a review from a team November 18, 2025 00:36
@eregon
Copy link
Member

eregon commented Nov 18, 2025

allow multiple benchmarks per libraries in the same folder

Some thoughts on this: maybe we could allow multiple run_benchmark calls per file and so have those benchmarks in the same file, but that has two problems: one benchmark might have effect on the next (and potentially make it less stable due to having e.g. a bunch of extra objects in the heap, polluted inline caches, etc), and it becomes hard to run a single benchmark.
So yeah, I think that feature would be nice.
One alternative I can think of would be to keep the file structure mostly as-is, but change category (per benchmark) to tags and then e.g. have an addressable tag, so we can have multiple overlapping "groups of benchmarks".

@rafaelfranca
Copy link
Collaborator Author

The problem with the current file structure that I found is that we have duplicated Gemfile and Gemfile.lock for the same libraries, which can cause benchmarks for the same library using different versions by mistake.

I like the suggestion about the tags as well, but I think we should build both.

This make easier to change the filter logic.
@rafaelfranca rafaelfranca force-pushed the rmf-extract-benchmark-filter branch from 0972ba1 to f837290 Compare November 18, 2025 19:25
@rafaelfranca rafaelfranca merged commit 15ad5ec into main Nov 19, 2025
3 checks passed
@rafaelfranca rafaelfranca deleted the rmf-extract-benchmark-filter branch November 19, 2025 18:18
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.

3 participants