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.

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

Demo issue #86 #107

Closed
wants to merge 1 commit into from
Closed

Conversation

arnaud-lb
Copy link

The purpose of this PR is to demonstrate issue sebastianbergmann/phpunit#4918 (I'm not requesting this to be merged).

HangingDemoTest executes Differ::diff() on two large strings. Differ::diff() hangs for hours in this case.

(The test skips itself if the CI environment variable is set, so that it doesn't consume github action quota.)

@sebastianbergmann
Copy link
Owner

Thank you for your contribution. I appreciate the time you invested in preparing this pull request. However, I have decided not to merge it.

@SpacePossum
Copy link
Contributor

Thanks for building the demo 👍
Do you get different result when using MemoryEfficientLongestCommonSubsequenceCalculator or TimeEfficientLongestCommonSubsequenceCalculator when calling diff?
I wonder if this an issue within both calculators or if something in selectLcsImplementation can be tweaked.

@arnaud-lb
Copy link
Author

arnaud-lb commented Mar 8, 2022

Yes, I'm getting different results depending on the calculator being used. MemoryEfficientLongestCommonSubsequenceCalculator is the one being used by default in this case (it's the one being choosen by selectLcsImplementation). I've tried with TimeEfficientLongestCommonSubsequenceCalculator, and it uses at least 4GB of memory (I haven't tried with a higher memory limit).

@SpacePossum
Copy link
Contributor

OK, yeah than the only solution I see would to be to develop another calculator for this use case. It is not something I can pick up any time soon, hope someone else might step forward.

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