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

Issue #10755: survived line in pitest.sh pitest-java-ast-visitor #10761

Merged
merged 1 commit into from Sep 1, 2021

Conversation

nrmancuso
Copy link
Member

@nrmancuso nrmancuso commented Aug 30, 2021

Closes #10755 . I ran pitest.sh pitest-java-ast-visitor in master branch five times on my local, and all failed as described in #10755. After reviewing the master branch's commit history, I cannot find any reason that this would start failing now after passing many times on my local and on github previously.

I have tested the increased number of concatenations in this PR 20 times on my local, and all have passed.

Link to successful pitest.sh pitest-java-ast-visitor run in this PR: https://github.com/checkstyle/checkstyle/pull/10761/checks?check_run_id=3461559365


From https://docs.oracle.com/javase/8/docs/api/java/lang/Thread.html:

Thread
public Thread(ThreadGroup group,
              Runnable target,
              String name,
              long stackSize)

Allocates a new Thread object so that it has target as its run object, has the specified name as its name, and belongs to the thread group referred to by group, and has the specified stack size.
This constructor is identical to Thread(ThreadGroup,Runnable,String) with the exception of the fact that it allows the thread stack size to be specified. The stack size is the approximate number of bytes of address space that the virtual machine is to allocate for this thread's stack. The effect of the stackSize parameter, if any, is highly platform dependent.

On some platforms, specifying a higher value for the stackSize parameter may allow a thread to achieve greater recursion depth before throwing a StackOverflowError. Similarly, specifying a lower value may allow a greater number of threads to exist concurrently without throwing an OutOfMemoryError (or other internal error). The details of the relationship between the value of the stackSize parameter and the maximum recursion depth and concurrency level are platform-dependent. On some platforms, the value of the stackSize parameter may have no effect whatsoever.

The virtual machine is free to treat the stackSize parameter as a suggestion. If the specified value is unreasonably low for the platform, the virtual machine may instead use some platform-specific minimum value; if the specified value is unreasonably high, the virtual machine may instead use some platform-specific maximum. Likewise, the virtual machine is free to round the specified value up or down as it sees fit (or to ignore it completely).

Specifying a value of zero for the stackSize parameter will cause this constructor to behave exactly like the Thread(ThreadGroup, Runnable, String) constructor.

Due to the platform-dependent nature of the behavior of this constructor, extreme care should be exercised in its use. The thread stack size necessary to perform a given computation will likely vary from one JRE implementation to another. In light of this variation, careful tuning of the stack size parameter may be required, and the tuning may need to be repeated for each JRE implementation on which an application is to run.

Implementation note: Java platform implementers are encouraged to document their implementation's behavior with respect to the stackSize parameter.

TLDR; The virtual machine is free to treat the stackSize parameter as a suggestion.


In light of the above information, I think that if pitest.sh pitest-java-ast-visitor continues to be flaky after this PR, we should allow suppression of failing lines in pitest.sh until #9267

@nrmancuso nrmancuso marked this pull request as draft August 30, 2021 12:14
@nrmancuso nrmancuso marked this pull request as ready for review August 30, 2021 14:06
@rnveach
Copy link
Member

rnveach commented Aug 30, 2021

I don't remember this anymore. If the stacksize limit was not in place, will InputJavaParserNoStackOverflowOnDeepStringConcat.java ever overflow naturally if the file kept growing to infinity? Is this a limit in ANTLR itself or somewhere else?

we should allow suppression of failing lines in pitest.sh until #9267

so you know, pitest.sh has no support for lines that come and go. We are expecting things to be stable.

@nrmancuso
Copy link
Member Author

We would like to make this input as large as we can, because we need the visitBinOp method's recursive AST building strategy to fail under pitest. If it does not fail, then the iterative strategy for AST building is not covered. We still need both strategies in this method, because child expressions may not always be binary operations.

We are expecting things to be stable.

Yes, I would just disable this test until #9267

Copy link
Member

@romani romani left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok to merge
There are better solution as noted, but we can use this meanwhile

@romani romani merged commit 9801c73 into checkstyle:master Sep 1, 2021
@nrmancuso nrmancuso deleted the issue-10755 branch September 1, 2021 18:27
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.

survived line in pitest.sh pitest-java-ast-visitor
3 participants