On Thu, 5 Oct 2023 16:29:26 GMT, Daniel Jeliński <djelin...@openjdk.org> wrote:

>> Please review this patch that removes a number of unused exports from 
>> java.desktop native libraries.
>> 
>> In most cases I removed JNIEXPORT from methods and variables that are only 
>> used within a single shared library. Other than that:
>> - removed `getSunFontIDs` that was reportedly used by rasterizer; as far as 
>> I could tell, rasterizer project is dead now, but if that's incorrect I can 
>> restore that export.
>> - removed `colorValueID` in X11Color; that field was not used.
>> - removed `J2dTraceInit` from header file. That method is only used 
>> internally by `J2dTraceImpl`.
>> 
>> The methods `Transform_GetInfo` and `Transform_transform` are declared in 
>> GraphicsPrimitiveMgr, but are only used in TransformHelper. Let me know if I 
>> should move them to where they are used.
>> 
>> The method `img_makePalette`, currently located in 
>> `share/native/libawt/awt/image/cvutils/img_colors.c`, is only used by 
>> `unix/native/common/awt/X11Color.c`; it could be moved to the same directory 
>> to avoid exporting the method from libawt. The files `img_colors.[ch]` do 
>> not have any references to other files in `cvutils`.
>> 
>> Manually verified that the exports are no longer present after these 
>> changes. Tier1-3 and client libs tests still pass.
>
> Daniel Jeliński has updated the pull request with a new target base due to a 
> merge or a rebase. The incremental webrev excludes the unrelated changes 
> brought in by the merge/rebase. The pull request contains four additional 
> commits since the last revision:
> 
>  - Merge remote-tracking branch 'origin' into desktop-exports
>  - Merge remote-tracking branch 'origin' into desktop-exports
>  - Make J2dTraceInit static
>  - Remove unnecessary exports

src/java.desktop/share/native/libawt/java2d/SurfaceData.h line 557:

> 555:  */
> 556: SurfaceDataOps *
> 557: SurfaceData_GetOpsNoSetup(JNIEnv *env, jobject sData);

~~It seems to me none of the functions in `SurfaceData.h` should be exported. 
They all end up in `awt.dll` / `libawt.so`. I can't see any of these functions 
are accessed from other DLLs.~~

~~Would you like to create a separate task for removing exports from 
`SurfaceData` functions?~~

Some are used from `fontmanager.dll`. But not as many as we export.

Potentially all symbols exported from `awt.dll` except from `Java*` can be 
hidden…  
I found that `Disposer_AddRecord` and `J2dTraceImpl` are used from `lcms.dll`.

src/java.desktop/share/native/libawt/java2d/Trace.c line 39:

> 37: JNIEXPORT void JNICALL
> 38: J2dTraceImpl(int level, jboolean cr, const char *string, ...)
> 39: {

`J2dTraceImpl` is used from `lcms.dll`, so it should remain exported. I thought 
it shouldn't be exported either.

src/java.desktop/unix/native/libawt/awt/initIDs.c line 47:

> 45:   (JNIEnv *env, jclass clazz)
> 46: {
> 47: }

Shall we remove `Color.initIDs`? Its implementation is now empty for all 
platforms.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/13261#discussion_r1355356940
PR Review Comment: https://git.openjdk.org/jdk/pull/13261#discussion_r1355354455
PR Review Comment: https://git.openjdk.org/jdk/pull/13261#discussion_r1355437407

Reply via email to