On Sat, 9 Apr 2022 06:02:13 GMT, Sergey Bylokhov <s...@openjdk.org> wrote:
>>> I don't understand why this was extracted out. It isn't called anywhere >>> else and again makes the diff harder to understand. >> >> This AWT lock should be held for a very small portion of the original >> method. Initially, I left the code guarded by it in place and it looked >> weird and out of place. I understand that this is a judgement call, though. >> >> As for the diffs, they are not very significant (essentially, 4 lines of >> code were re-grouped and moved around) and we only have to look at them here >> for a few days. The (refactored) code itself will be looked at by >> (presumably) a greater number of people over a far greater period of time. >> Less of a judgement call. > >>This is just a named copy of lines 67 to 75. Since now this is the only piece >>of code that needs to work holding the AWT lock, I thought it best to isolate >>it. > As I understand, the purpose of the piece is to find the "new" > X11GraphicsDevice with the screen number same to the given "old" > GraphicsConfiguration. > > I am not sure that we can have some "old" and "new" devices which have the > same screen id. > * If the list of devices was not changed then the "screenid" for old device > is the index of that "old" device in the device array. So the new and old > devices are the same. > * If the list of devices was changed, then the "screenid" for the old device > was invalidated, and it is again an index of that "old" device in the device > array. Also I do not think that we can change the "requested device" in this > method, The purpose of this method is to request the best "GC" from the > device passed as a parameter(via another GC). > > Please take a look to the fix for JDK-6804747, initially that method get the > screen index as a parameter, and then replaced by the GC. The code of the > method did not changed but the index just requested from the GC. > > Probably the code is just buggies? @mrserb I agree with your logic, but fixing this now would certainly be not a semantically null change and would complicate the fix that has already been under review for over two months. I'd be happy to file a bug for this separate issue. ------------- PR: https://git.openjdk.java.net/jdk/pull/7182
