Re: RFR: 8286681: ShenandoahControlThread::request_gc misses the case of GCCause::_codecache_GC_threshold [v2]
On Fri, 13 May 2022 12:27:16 GMT, Zhengyu Gu wrote: > LGTM Thanks @zhengyu123 for the review. - PR: https://git.openjdk.java.net/jdk/pull/8691
Re: RFR: 8286681: ShenandoahControlThread::request_gc misses the case of GCCause::_codecache_GC_threshold [v2]
> Hi all, > > Some tests fail with Shenandoah GC after JDK-8282191. > The reason is that the assert in `ShenandoahControlThread::request_gc` misses > the case of `GCCause::_codecache_GC_threshold`. > It would be better to fix it. > > Thanks. > Best regards, > Jie Jie Fu has updated the pull request incrementally with one additional commit since the last revision: Revert the test change - Changes: - all: https://git.openjdk.java.net/jdk/pull/8691/files - new: https://git.openjdk.java.net/jdk/pull/8691/files/8f094d32..dc96246c Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8691=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8691=00-01 Stats: 15 lines in 1 file changed: 0 ins; 15 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/8691.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8691/head:pull/8691 PR: https://git.openjdk.java.net/jdk/pull/8691
Re: RFR: 8286681: ShenandoahControlThread::request_gc misses the case of GCCause::_codecache_GC_threshold [v2]
On Fri, 13 May 2022 13:18:55 GMT, David Holmes wrote: >>> I think I agree with @AlanBateman - in the sense that this seems to go down >>> a slippery slope where every test would need to be executed against all >>> possible GCs. AFAIK, there are no other foreign tests doing this. >> >> I think it's a waste of time to write a separate test for this bug. >> >> I can't understand why you are against adding one more test config in the >> foreign test. >> Yes, it caught the bug in ShenandoahGC but who knows it wouldn't find some >> other bugs in the foreign api in the future. >> If you are still against the newly added test config, I can revert the test >> change. >> Thanks. > > My comment seems to have never made it through. The problem with what your > are doing is two-fold: > 1. When running the test suite specifically to test xGC for whatever x you > are now forcing a test run for Shenandoah. > 2. When x is Shenandoah then you run this test twice. > > You don't need a config just to run this with Shenandoah - you run the test > suite with Shenandoah. The test change had been reverted. Thanks. - PR: https://git.openjdk.java.net/jdk/pull/8691
