Skip to content

Deduplicate LocalTime#285

Open
leonardehrenfried wants to merge 4 commits intoentur:masterfrom
leonardehrenfried:deduplicate-local-time
Open

Deduplicate LocalTime#285
leonardehrenfried wants to merge 4 commits intoentur:masterfrom
leonardehrenfried:deduplicate-local-time

Conversation

@leonardehrenfried
Copy link

@leonardehrenfried leonardehrenfried commented Mar 18, 2026

This implements deduplication of instances of LocalTime.

This is okay, because it's an immutable value object where one instance truly can be replaced with an equal one.

Improvement

In my current example file this can cut down the number of instances from around 30 million to a few thousand, saving around 800MB of RAM!

cc @vpaturet

Ref: #284
Ref: noi-techpark/opendatahub-mentor-otp#273

cc @rcavaliere

@vpaturet vpaturet self-requested a review March 18, 2026 14:55
Copy link
Contributor

@vpaturet vpaturet left a comment

Choose a reason for hiding this comment

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

Looks good.
Could you document:

  • thread safety: why it works.
  • memory leak: why this is a limited risk.

}

@Test
public void testCaching() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you add a test with different variants like: "10:20:30" vs "10:20:30+02:00"?

Copy link
Author

Choose a reason for hiding this comment

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

Done.

@leonardehrenfried
Copy link
Author

I'm actually not sure how many threads are using the adapter concurrently. If there is a chance that it's multiple, we can synchronize the map map or use a concurrent one. Any opinions?

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.

2 participants