On Tue, 29 Sep 2020 10:02:16 GMT, Erik Österlund <eosterl...@openjdk.org> wrote:
>> IMO it's ok to remove it. >> However, it can be argued that is_in() should not check for 'in committed >> memory' but 'in reserved space for heap', IOW >> 'is this a pointer into our heap memory region?'. Or maybe this would be >> CH::is_in_reserved() and we should change the >> assert to that? Also, IMO we should not do anything that may trip barriers >> on oop-iterators or similar GC-only paths. > > Almost always, what you want to do, is to assert the validity of heap > pointers, where they are expected to be valid > (dereferenceable pointer to a not freed object). And then you actually do > want to check that they are within the > committed boundaries of the heap. Otherwise you have a bug. We don't want to > make such reasonable assertions weaker, > because we cater for asserts that want oops to look valid pretty much all the > time, even when they are stale pointers > from a previous cycle, pointing into freed memory (until it is fixed). And I > would much rather cater for asserts with > good underlying reasoning (the oop should actually be okay here or something > is wrong), than to cater for asserts that > seem to just have the wrong underlying reasoning, making everyone else suffer > for it, and letting bugs slip past all > the other asserts. The is_in_reserved check no longer exists. And I don't > think I want to bring it back from the dead > for this assert. Because I don't think the assert is right. The GCs already > have their own verification code for roots > (and the heap), that does this job, but much better. So I don't think it > makes sense for the actual shared code oop > iterator to go in and put constraints on what an invalid (not yet fixed up) > stale pointer should and should not look > like, for all GC implementations. Let the GC's own verification code take > care of that. It has a much better idea about > what a stale pointer should and should not look like. And I do agree that we > should not use barriers in this kind of > code path. I just didn't know what else to do here to make the assert happy. > Sorry about that. Well now I know. I am > going to delete it, unless anyone opposes that solution. It's ok for me. ------------- PR: https://git.openjdk.java.net/jdk/pull/296
