Skip to content

Module observability listener performance issue on publishEvent #1149

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Groax256 opened this issue Apr 14, 2025 · 2 comments
Open

Module observability listener performance issue on publishEvent #1149

Groax256 opened this issue Apr 14, 2025 · 2 comments
Assignees
Labels
in: observability Observability support meta: waiting for feedback Waiting for feedback of the original reporter type: improvement Minor improvements

Comments

@Groax256
Copy link

In org.springframework.modulith.observability.ModuleEventListener#onApplicationEvent search for module by type uses simpleName which means it cannot short-circuit condition in org.springframework.modulith.core.ApplicationModule#contains(java.lang.String) with package name. The search is implemented on streams so it does not scale well with increasing number of classes. As a result it has to scan hundreds of classes with ArchUnit on each publishEvent call.

There are 21 modules and 1180 classes in my project and on publishEvent there is significant perfomance hit from ModuleEventListener.

Image

I use Modulith 1.3.1 but based on code source the issue will be the same in 1.4.0

@odrotbohm
Copy link
Member

I don't recall why we are looking up the module by the String type name. The only thing I can imagine is that at the time, the methods taking JavaClass and Class instances were introduced.

Unfortunately, the lower layers all immediately flip to the by-name code path, most likely to end up at a canonical place. We can certainly fix that, but ultimately, an initial lookup will have to traverse all modules and all types contained in them. I am also hesitant to build up caches on the ApplicationModules level as that might grow unboundedly.

That said, a local cache in ModuleEntityListener might be a good compromise, as usually the number of event types is much smaller than all application types combined. Plus, aside from the application startup and shutdown, they're very likely to be application (module) types in the first place.

WDYT?

@odrotbohm odrotbohm self-assigned this Apr 17, 2025
@odrotbohm odrotbohm added in: observability Observability support type: improvement Minor improvements meta: waiting for feedback Waiting for feedback of the original reporter labels Apr 17, 2025
odrotbohm added a commit that referenced this issue Apr 17, 2025
We now avoid immediately resorting to a by type name lookup that triggers extensive search to be able to support simple class names.
odrotbohm added a commit that referenced this issue Apr 17, 2025
…ener.

To avoid repeated, expensive full lookups across the entire class space for each listener invocation.
@odrotbohm
Copy link
Member

Feel free to give the 1.4 snapshots a try. If this improves the situation, I can backport this into 1.3.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: observability Observability support meta: waiting for feedback Waiting for feedback of the original reporter type: improvement Minor improvements
Projects
None yet
Development

No branches or pull requests

2 participants