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.

Sign up for GitHub

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

Jump to bottom

include impacts performance #287

Open
TobiasNx opened this issue Mar 6, 2023 · 7 comments
Open

include impacts performance #287

TobiasNx opened this issue Mar 6, 2023 · 7 comments
Assignees
@blackwinter
Labels
Bug Something isn't working Enhancement New feature or request Metafix

Comments

@TobiasNx
Copy link
Collaborator

TobiasNx commented Mar 6, 2023

A fix transfomation in one file is faster and needs less storage when executed than a fix transformation that is distributed over multiple files and combined via include-functions.
@TobiasNx TobiasNx added the Enhancement New feature or request label Mar 6, 2023
@blackwinter
Copy link
Member

Previous chat discussion (in German).

BTW: I would consider this a bug, but that's certainly debatable.

@TobiasNx TobiasNx added the Bug Something isn't working label Mar 6, 2023
@dr0i dr0i added this to Metafacture Mar 6, 2023
@dr0i dr0i moved this to Ready in Metafacture Mar 6, 2023
@dr0i
Copy link
Member

dr0i commented Mar 27, 2023

Fixing this may help fixing hbz/lobid-resources#1666.

blackwinter added a commit that referenced this issue Oct 20, 2023
@blackwinter
Add "integration tests" to benchmark include() performance. (#287)
4ddfcac 
- includeBenchmarkBase1:       3.2375 +/- 0.0193
- includeBenchmarkBase1000:    3.5835 +/- 0.1132
- includeBenchmarkBase100000:  5.4308 +/- 0.1477
- includeBenchmarkSome1:       3.6988 +/- 0.0185
- includeBenchmarkSome1000:    3.9415 +/- 0.2000
- includeBenchmarkSome100000:  7.8708 +/- 0.1882
- includeBenchmarkMany1:       5.1312 +/- 0.0764
- includeBenchmarkMany1000:    5.5572 +/- 0.2899
- includeBenchmarkMany100000: 17.6438 +/- 0.0740
blackwinter added a commit that referenced this issue Oct 20, 2023
@blackwinter
Cache resolved Fix file path to improve include() performance. (#287)
4fb8426 
- includeBenchmarkBase1:      3.3080 +/- 0.0558
- includeBenchmarkBase1000:   3.4018 +/- 0.0212
- includeBenchmarkBase100000: 5.4032 +/- 0.0494
- includeBenchmarkSome1:      3.7932 +/- 0.1135
- includeBenchmarkSome1000:   3.8172 +/- 0.0428
- includeBenchmarkSome100000: 6.1055 +/- 0.1947
- includeBenchmarkMany1:      5.2958 +/- 0.2262
- includeBenchmarkMany1000:   5.3335 +/- 0.3680
- includeBenchmarkMany100000: 7.7365 +/- 0.1504
@dr0i
Copy link
Member

dr0i commented Oct 30, 2023

Don't know if this can be closed @blackwinter ? With #327 we gained at least some performance.

@blackwinter
Copy link
Member

Ultimately, @TobiasNx has to decide whether his use case has improved sufficiently. I didn't link the pull request to this issue intentionally.

The next step would need to be to look at the RecordTransformer implementation itself, which would potentially also affect the main Fix and call_macro() in addition to include().

@dr0i dr0i assigned TobiasNx and unassigned blackwinter Oct 30, 2023
@TobiasNx
Copy link
Collaborator Author

TobiasNx commented Nov 7, 2023

It seems that it got 30 min faster.

@blackwinter
Copy link
Member

It seems that it got 30 min faster.

That's roughly 3.5 %.

@blackwinter blackwinter changed the title include impacts perfomance include impacts performance Nov 7, 2023
@dr0i dr0i mentioned this issue Nov 20, 2023
Merged
@TobiasNx
Copy link
Collaborator Author

TobiasNx commented May 6, 2024

In my opinion the first step is done and it improved the performance.
Next step would be the suggested rework of RecordTransformer

@dr0i dr0i assigned blackwinter and unassigned TobiasNx Jul 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@blackwinter blackwinter

Labels
Bug Something isn't working Enhancement New feature or request Metafix
Projects
Metafacture
Status: Ready
Milestone
No milestone
Development

No branches or pull requests
3 participants
@blackwinter @dr0i @TobiasNx