Skip to content

Conversation

@Technici4n
Copy link
Contributor

I noted that building the module references was quite slow on the FC pack, so I propose these two simple changes:

Performance numbers on the FC pack (single run per line so not super precise but VisualVM confirms the observations):

Baseline: 4292ms
Added root package filter without FML support: 4207ms
Added root package filter with FML support: 1136ms
Additionally make jar resolution parallel: 340ms

Copy link
Member

@marchermans marchermans left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In principal this is fine. But I have one thing to nitpick

@ichttt
Copy link
Member

ichttt commented Aug 22, 2023

Just out of curiosity: Can you run the JarModuleFinderBenchmark? I know the set of jars is not optimal for testing the speedup of parallel scanning, but I think they still provide some value.
I like the idea of blacklisting directories that should not contain any code, but maybe we should add something in the Build Pipeline or userdev that warns if classes are present in these Directories, as it could lead to confusion why some stuff doesn't work otherwise. But tbh that is probably not needed, as it would be very weird to store classes there.

Overall, nice work!

@Technici4n
Copy link
Contributor Author

Parallel scanning is only useful for jar metadatas that lazily compute their packages. The metadata implementations in SJH compute it eagerly (but ModJarMetadata computed it lazily and hence benefits from this speedup).

As you can see here the main hit from JarModuleFinderBenchmark is getPackages, which is not parallel with the default metadata:
image

@Technici4n
Copy link
Contributor Author

Technici4n commented Aug 25, 2023

Default jar metadata implementations should be changed too to benefit from parallelization, that might save over 1s of startup in some cases based on some profiles from ATM8.

@Technici4n Technici4n marked this pull request as draft August 25, 2023 09:23
@Technici4n
Copy link
Contributor Author

Superseded by #47 and #57.

@Technici4n Technici4n closed this Nov 27, 2023
@Technici4n Technici4n deleted the faster-jar-handler branch November 27, 2023 08:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants