Skip to content

[SCB-360] write simpler eventBus to optimize performance#570

Merged
liubao68 merged 1 commit into
apache:masterfrom
wujimin:eventbus
Mar 15, 2018
Merged

[SCB-360] write simpler eventBus to optimize performance#570
liubao68 merged 1 commit into
apache:masterfrom
wujimin:eventbus

Conversation

@wujimin
Copy link
Copy Markdown
Contributor

@wujimin wujimin commented Mar 5, 2018

IDE compile warnings check:
image

Comment thread foundations/foundation-common/pom.xml Outdated
</dependency>
<dependency>
<groupId>com.google.code.findbugs</groupId>
<artifactId>jsr305</artifactId>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

why add this dependency? Can not find it's usage from code below

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

my first impl extend EventSubscriber to write a new Subscriber, if not add the dependency, will cause class not found exception
but impl changed at last, i will if need it too.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

no need this anymore, delete it.

}

@Override
public void post(Object event) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This implementation will block event senders if subscribers processed not very fast.
It's better to mention this in the class description.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

registrer and unregister will not block post

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

post action follow guava definition, just make it simpler(see comments in class level)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

OK. This will just make events out of order when trigger events in events handling logic.

// different between this class and EventBus:
// 1. not support cycle trigger:
// subscribe a, when handle a, trigger event b
// subscribe b, when handle b, trigger event a
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It's better to make this comments as Java Docs in EventManager in case user miss use it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

changed to javadoc in SimpleEventBus
but not in EventManager

@wujimin wujimin changed the title SCB-360 write simpler eventBus to optimize performance [SCB-360] write simpler eventBus to optimize performance Mar 7, 2018
@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage decreased (-0.004%) to 87.226% when pulling 1c474b0 on wujimin:eventbus into 6f62b75 on apache:master.

@liubao68 liubao68 merged commit eca2b3d into apache:master Mar 15, 2018
@wujimin wujimin deleted the eventbus branch April 2, 2018 00:47
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