-
Notifications
You must be signed in to change notification settings - Fork 580
testuite.yml - add PERL_SAWAMPERSAND testing #21304
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
base: blead
Are you sure you want to change the base?
Conversation
PERL_SAWAMPERSAND is related to how perl handles the use of $`, $& and $' and the behavior of the regex engine. When it is defined then certain slow copy operations are avoided UNLESS the code uses one of these magic variables. This can have drammatic performance consequences for certain code patterns, for example in while (//g) matching against long strings. When COW strings were introduced it was assumed that COW would be a better solution to pessimizing the regex engine when these variables were used, and for many code cases it is indeed more efficient. However it is not a universal and consistent solution and there are cases where matches that would be linear under PERL_SAWAMPERSAND are quadratic without it. For those cases we support building perl with PERL_SAWAMPERSAND defined so we do not rely on COW to deal with these copy operations alone. Since the default build is to not define PERL_SAWAMPERSAND code that is governed by this build mode is not tested under normal circumstanctes. This patch ensures that we test that code as well as the default build mode. From time to time people suggest that we remove PERL_SAWAMPERSAND and simply rely on COW, however I believe that this is unwise. COW does not apply to all strings, and COW only supports 254 copies of a string, so some operations actually would end up quadratic. The situation is unsatisfactory IMO, and until we can make COW more powerful than it is I believe we should continue to support the escape hatch represented by PERL_SAWAMPERSAND which implies testing it.
Can you elaborate (in the commit message) on why it's important to test this partciular configuration all the time? |
273787c
to
b53680b
Compare
@bram-perl I think we should be testing all of our optional build modes all the time, so to me this is like asking "why do you bother with CI". But I have updated the commit regardles. |
On 7/27/23 13:48, Yves Orton wrote:
@bram-perl <https://github.com/bram-perl> I think we should be testing
all of our optional build modes all the time, so to me this is like
asking "why do you bother with CI". But I have updated the commit regardles.
Testing all is impractical. For my locale changes, I have over 200
different combinations of build options. This is because platforms vary
greatly in their locale handling
|
@khwilliamson The ideal is often impractical. My point was that I shouldn't have to explain why I am adding one to be tested. If we had infinite resources we would test them all. It is your call if you want to test your options or not. That you choose not to test some doesn't mean I should have to explain why I am testing another. |
The point of explaining it (in the commit message) is so that people later know why it was done. Let's assume that there is no reason in the commit message and that someone in the future decides to remove PERL_SAWAMPERSAND: then they'll notice that the testsuite fails and just remove the test configuration. After all there is no one that remembers why it was added in the first place. |
Make sure we test this build mode.