improve parser performance

[dcendents]

Hi,

Using py-spy to check where to improve the performance of my tests, I noticed a lot of time was spent in the Step name property.
Just by caching the value instead of recalculating it every time I gain ~15%.
Same gain when I run the tests in parallel

Sequence:

• Before:
  • 1280 passed, 49 deselected in 125.52s (0:02:05)
• After:
  • 1280 passed, 49 deselected in 107.67s (0:01:47)

Parallel:

• Before:
  • 1280 passed in 60.61s (0:01:00)
• After:
  • 1280 passed in 50.24s

It is a very minor change, hope it can be merged.

Thanks

[youtux]

hi, interesting find. I think it may have been already a cached property in the past, but the for some reason I must have removed it.

Could you change the implementation by using functools.cached_property instead? It should result in a way neater implementation.

Also, make sure to invalidate the cached attribute when self.lines or self._name are modified.

[codecov]

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.05 🎉

Comparison is base (73c2393) 95.40% compared to head (c52a5fa) 95.45%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #623      +/-   ##
==========================================
+ Coverage   95.40%   95.45%   +0.05%     
==========================================
  Files          49       49              
  Lines        1763     1783      +20     
  Branches      160      195      +35     
==========================================
+ Hits         1682     1702      +20     
  Misses         53       53              
  Partials       28       28              

Impacted Files	Coverage Δ
src/pytest_bdd/parser.py	98.68% <100.00%> (+0.05%)	⬆️
tests/steps/test_common.py	100.00% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

[dcendents]

thanks for the review, I have modified the code to use cached_property.
However there is no setter on a cached_property so I still had to create a new property to keep name as is.

Performance is the same

Let me know what you think

[dcendents]

Hi @youtux ,

Actually you were right we can del self.full_name but we do need to validate the key is in __dict__ first, which I was not doing the first time around and caused the error I mentionned.

Anyway that showed my modifications were lacking proper testing so I added a unit test as well to validate the cache behavior.
I really tried to use a mock to wrap the step object and spy on the full_name method (property) but I could not make it work, so I hope you are ok with the way I tested it.

Changing values directly on the step object to show the cache value is still returned (which should never happen in production code).
Changing values using properties show the cache is invalidated.

Cheers