Closed gerlero closed 1 year ago
Merging #73 (40de7da) into master (3ec3b15) will decrease coverage by
0.17%
. Report is 1 commits behind head on master. The diff coverage is100.00%
.:exclamation: Current head 40de7da differs from pull request most recent head eb4b3fe. Consider uploading reports for the commit eb4b3fe to get more accurate results
@@ Coverage Diff @@
## master #73 +/- ##
==========================================
- Coverage 87.22% 87.06% -0.17%
==========================================
Files 5 5
Lines 321 317 -4
==========================================
- Hits 280 276 -4
Misses 41 41
Files Changed | Coverage Δ | |
---|---|---|
src/types.jl | 100.00% <100.00%> (ø) |
:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more
FWIW, I've also tried the code from #67 but this change doesn't seem to affect that at all.
@Krastanov I understand what JET is complaining about, but I'm not sure what's the right way to make it happy.
What are your thoughts on adding a dummy definition of (::FiniteStateMachineIterator)(...)
that throws?
This looks good to me. Let me merge another PR that adds the benchmarks to CI, then I will rerun the CI here and merge.
I will look into the JET report, but unless there is a very clean non-hacky fix, I would just bump the number of disregarded warnings.
Quick fix for the performance regression reported in #72: reverts the changes to the definition of the
Base.iterate
overload, no longer delegating togenerate
, which is causing unnecessary allocations.Before (
master
)After
Closes #72. Note that the
generate
/@yieldfrom
paths still cause many allocations, but (a) they're not part of the benchmarks and (b) I haven't been able to come up with a fix for that yet.