laradumps / laradumps

🛻 LaraDumps is a friendly app designed to boost your Laravel PHP coding and debugging experience.
https://laradumps.dev
MIT License
743 stars 39 forks source link

[BUG] Upgrade V3 huge slowdown in tests execution #147

Closed adrien-delhom closed 7 months ago

adrien-delhom commented 7 months ago

Pre-steps

Have you searched through other issues to see if your problem is already reported or has been fixed?

Did you read the documentation?

Have you tried to reconfigure LaraDumps?

To configure LaraDumps, run: php artisan ds:init.

Software Version

You can run composer show -i and npm list to list installed package with their versions.

Software Version (exactly)
LaraDumps App 3.0.2
LaraDumps package v3.0.0
Laravel v10.48.4
Operating System Macbook Pro M1 Sonoma 14.3

What is your project Environment?


Describe the bug

What happened?

We've decided to use Laradump rather than Laravel Ray at work, and we're very happy with it. A little problem: I upgraded the package (dev) to V3 on our API, no problem locally, but our CI (tests) went from 10mn to 35mn. I tried to identify the source, but nothing could be done. I also noticed this latency in local, I've had to downgrade it for now.

Steps to Reproduce the problem

Suggestions

EDIT: Not really good long term solution, but for the moment we remove the package in our CI

Extra information

Sorry, it's private information, I can't give more detail...

Screenshots After the upgrade image

After the revert image

luanfreitasdev commented 7 months ago

Hello!

Thank you for reporting this issue

I'm investigating the cause of this. Can you install this version and test it on your CI? It would be great

composer require laradumps/laradumps --dev dev-147-bug-upgrade-v3-huge-slowdown-in-tests-execution
adrien-delhom commented 7 months ago

@luanfreitasdev we appreciate your responsiveness!!

Our normal CI time with my "tricks" (remove laradumps in CI) image

CI time with your custom branch (not finished yet, achieve 60% of tests ~) image

I don't know if you want to spend more time on this problem, because in the end few users will be impacted, but I think it raises a small concern that may be more "global" to our case.

Recommendation: I think your package creates a CPU overload somewhere (maybe via Buses or Listener?), which adds a few ms to each of our tests, so if someone uses the package in production it will have an impact if my theory is right.

For that, I'd recommend several approaches:

To reproduce IC conditions:

luanfreitasdev commented 7 months ago

Right. This doesn't make sense because all observers are disabled by default unless there is some ds() in the path.

No dependency could cause this

luanfreitasdev commented 7 months ago

Is there any dump() in the files?

adrien-delhom commented 7 months ago

Nop, no dd, dump or anything, we have rule to reject that in production

luanfreitasdev commented 7 months ago

Could you at some point test again with the latest commit? I would love to understand this and fix it.

adrien-delhom commented 7 months ago

I'll test tomorrow morning ;)

luanfreitasdev commented 7 months ago

I tested it on 3 projects with large tests and I didn't have this behavior

adrien-delhom commented 7 months ago

Are you talking with your fix or with the release version of the package? Sorry I don't have a lot of time this morning to make the test with CI and your custom branch