Skip to content
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

IGNITE-24163 Optimize EventLog #5004

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

PakhomovAlexander
Copy link
Contributor

@PakhomovAlexander PakhomovAlexander commented Jan 7, 2025

  • Get rid of unnecessary locking in log method
  • Add one more "lazy" log method to the interface
  • Return null instead of empty collection (isEmpty() call was contributing about 20% execution time to EventLog.log method)

https://issues.apache.org/jira/browse/IGNITE-24163

@PakhomovAlexander PakhomovAlexander force-pushed the IGNITE-24163 branch 2 times, most recently from 43c78cc to 0adaae8 Compare January 7, 2025 15:52
@PakhomovAlexander PakhomovAlexander marked this pull request as draft January 7, 2025 15:52
guard.readLock().unlock();
}
Set<EventChannel> result = typeCache.get().get(igniteEventType);
return result == null ? Set.of() : new HashSet<>(result);
Copy link
Member

Choose a reason for hiding this comment

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

No need to copy result.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why? I return the collection outside and I do not want anybody to modify it.

@PakhomovAlexander
Copy link
Contributor Author

Screenshot 2025-01-08 at 19 01 07 Benchmark results against baseline (single if statement).

@PakhomovAlexander PakhomovAlexander force-pushed the IGNITE-24163 branch 2 times, most recently from e48737e to 9ac4c34 Compare January 8, 2025 16:06
* Get rid of unnecessary locking in `log` method
* Add one more "lazy" log method to the interface
@PakhomovAlexander PakhomovAlexander marked this pull request as ready for review January 8, 2025 16:10
@PakhomovAlexander
Copy link
Contributor Author

PakhomovAlexander commented Jan 8, 2025

@AMashenkov applied most of your suggestions. Also, added a couple of additional optimizations (return null instead of empty collection) wich have improved the performance of the critical path.

The PR now is ready for review.

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