On Wed, 14 Feb 2024 18:09:15 GMT, Per Minborg <pminb...@openjdk.org> wrote:
>> This PR proposes to remove the use of `Unsafe` in the class >> `XEmbeddingContainer ` and replace it with the supported FFM API. >> >> I tried to make this PR as small as possible while opening up for migration >> of other classes later on (such as `XEmbedServer` which can be modified >> similarly under a separate PR). >> >> There are also three drive-by fixes in this PR: >> * Moved JavaDocs for `XAtom` to its proper location and fixed two typos in >> the text >> * Declared a field in `XEmbeddingContainer` as `private final` >> * In `XAtom`, use a utility method `assertAtomInitialized()` instead of the >> current duplicated code >> >> Tested and passed tier1-5 > > Per Minborg has updated the pull request incrementally with one additional > commit since the last revision: > > Suppress restricted warning In the interests of gauging what the actual impact might be (performance-wise), I tried looking for where we might create an XEmbeddingContainer() and I can find absolutely none. Making the sole constructor throw "new Error(..)" doesn't seem to cause any problems at all. This looks like dead code, unless someone else knows why we still have it. The constructor XEmbedHelper() is called from some test code which is *in the source tree* not in a test. However at least static fields and perhaps methods on that class are used. But my conclusion from what I can see is that although the changes in XAtom seem like they could be re-used, the best thing to do with XEmbeddingContainer might be to delete it. Anyone have ANY idea where or how this might be used ? I'd like to be sure before I propose that. FWIW the changes all look perfectly fine to me .. although I am curious why a reachabilityFence was needed in one place. And the XAWT code has many uses of Unsafe.allocateMemory which could be candidates, but I assume that the timeline for removal of jdk.internal.misc.Unsafe (which is what this code uses) does not have to be the same as that for sun.misc.Unsafe memory allocation methods. ------------- PR Comment: https://git.openjdk.org/jdk/pull/17846#issuecomment-1955246166
