hstaudacher / osgi-jax-rs-connector

An OSGi - JAX-RS 2.0 Connector, software repository available on the link below
http://hstaudacher.github.io/osgi-jax-rs-connector
Other
190 stars 98 forks source link

Performance optimizations #92

Closed kolkoo closed 9 years ago

kolkoo commented 9 years ago

I've identified several things I intend to fix in a pull request soon that affect performance especially on slower embedded devices. Firstly I'm going to say that Jersey initialization/reload is really slow on the particular embedded device I'm testing with, so in light of that there are several problems:

1) ResourceTracker - in the addingService method if a service does not have jax-rs annotations we should unget the service and return null in order to not track the service anymore. This also allows us to remove the isResource() checks in the {removed,modified}Service methods.

2) Currently the connector is tracking ApplicationConfiguration, ServletConfiguration and also ConfigAdmin Configuration, when these three appear the way things are handled a new JerseyContext is created - so on a slower device we may end up with up to 4 Jersey initializations as follows: 2.1) Once when first resource appears 2.2) Another one when ConfigAdmin Configuration appears 2.3) Another one when ApplicationConfiguration appears 2.4) And a last one when ServletConfiguration appears On a device where Jersey configuration is really slow (10-20 seconds) this is quite a heavy hit. I intend to optimize this by including Configuration/ApplicationConfiguration/ServletConfiguration into the same/similar mechanism that is used by Resources (using publishDelay).

3) This is related to 2.1). When the first resource appears we register our servlet with OSGi - which is fine but this currently causes Jersey ServletContainer.init() to be called which initializes jersey, then after resources all appear we reload jersey - so at minimum two Jersey loads in most cases. Another problem is that while Jersey is reloading asynchronously we could try to access our servlet and we get a nasty exception from Jersey while that is happening. I intend to have the ServletContainerBridge extend HttpServlet and wrap the Jersey ServletContainer, so if Jersey is loading/reloading we can return Error 503 not available for example.

4) In relation to 3) I think it's good to have some sort of listener/hook mechanism before and after the Jersey Asynchronous initialization is performed. I currently need to include some device specific code to disable security checks for the Jersey thread and would also be good to know when Jersey is fully ready.

5) Possibly also include a way to delay initialization until it is manually triggered. This way I can trigger Jersey initialization at a time where all my resources should already be registered and avoid most of the problems described in 2). Of course if no such trigger is present we would fall back to the default behavior.

Would really appreciate if you let me know whether or not you feel this is something you would consider merging as a heads up :)

Thanks in advance,

Ivan

hstaudacher commented 9 years ago

Hi sounds good. Here is some feedback:

1) Checking for the annotation is not enough. We can also register Feature types. 2) Sounds reasonable. Looking forward to see how you handle it ;) 3) Sounds creepy but ok. 4) Are you planing to have those hooks available as API? If yes we need to follow the whiteboard pattern. 5) Hmm, ok ;)

What would really help if you can do a separate pull request for each topic because the changes sound big.

kolkoo commented 9 years ago

Hi again, 1) I meant your isResource method which also checks for instanceof Feature. I've decided against 5) as it is quite tricky to implement and may not be that helpful if 2 and 3 are already in place. As for 4) I'd use the whiteboard pattern but I'm still not sure if the feature is needed at all.

I'm not sure I can separate everything easily but I'll discuss it and get back to you :)

kolkoo commented 9 years ago

Alright so 4) and 5) are going to drop out of this PR. 5) I consider unnecessary after some research.

As for 4) I can already use Jersey's ApplicationEventListener as an OSGi Service with @Provider annotation and that works great :)

kolkoo commented 9 years ago

I have one question, if a resource is removed the application is not marked as dirty -> https://github.com/hstaudacher/osgi-jax-rs-connector/blob/c03759e5fbd452617ebbba9cba7a57381257e495/bundles/com.eclipsesource.jaxrs.publisher/src/com/eclipsesource/jaxrs/publisher/internal/RootApplication.java#L46

why is that? Shouldn't jersey context be reloaded when we are removing resources?

hstaudacher commented 9 years ago

Definitely. Seems you found a bug.

kolkoo commented 9 years ago

Ok I will include the fix in this PR as well :)

hstaudacher commented 9 years ago

Alright, can you also add a testcase for this? It seems it was missing.

kolkoo commented 9 years ago

Yes, I will, for this PR I'll have to remove, add and rework a lot of tests anyway.

kolkoo commented 9 years ago

Do you happen to have an eclipse formatter for the publisher bundle? I can see that there is a custom one defined in the project settings but for some reasons it messes up your license headers + javadocs and removes any emtpy lines within the code? Is this intended?

hstaudacher commented 9 years ago

No, it's not intended to mess it up :)

Just use the format you are used to. I will go over the change anyway ;). I'm kind of a paranoid developer...

kolkoo commented 9 years ago

I've just created the PR. Unfortunately I was not able to separate the issues easily. Hopefully the changes are not too much.

https://github.com/hstaudacher/osgi-jax-rs-connector/pull/93