RFR: 8316421: libjava should load shell32.dll eagerly [v2]

Jorn Vernee jvernee at openjdk.org
Wed Sep 20 14:45:44 UTC 2023


On Mon, 18 Sep 2023 18:10:12 GMT, Daniel Jeliński <djelinski at openjdk.org> wrote:

>> Please review this patch that makes java.dll load shell32.dll earlier. Delay-loading requires some additional code (delayimp.lib), and offers no benefits since we always load shell32 during JVM startup.
>> 
>> Other than removing the delayload clause, the patch also cleans up the `getHomeFromShell32` method:
>> - the WinXP code path is removed. The documentation states that [the older function just calls the new one with translated parameters](https://learn.microsoft.com/en-us/windows/win32/api/shlobj_core/nf-shlobj_core-shgetfolderpatha).
>> - `CoTaskMemFree` is now called if `SHGetKnownFolderPath` fails. This is suggested by the documentation, but probably never needed in practice.
>> 
>> This change shouldn't have any observable effect on behavior on any of the supported operating systems. It reduced the size of java.dll by 2KB on my machine.
>> No new tests. Existing tier1-2 tests continue to pass.
>
> Daniel Jeliński has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Remove unused define

LGTM

src/java.base/windows/native/libjava/java_props_md.c line 214:

> 212:         HRESULT hr;
> 213:         WCHAR *tmpPath = NULL;
> 214:         hr = SHGetKnownFolderPath(&FOLDERID_Profile, KF_FLAG_DONT_VERIFY, NULL, &tmpPath);

I'd say inline the variable declaration here as well
Suggestion:

        HRESULT hr = SHGetKnownFolderPath(&FOLDERID_Profile, KF_FLAG_DONT_VERIFY, NULL, &tmpPath);

-------------

Marked as reviewed by jvernee (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/15789#pullrequestreview-1635822883
PR Review Comment: https://git.openjdk.org/jdk/pull/15789#discussion_r1331744667


More information about the build-dev mailing list