owlcs / owlapi

OWL API main repository
828 stars 315 forks source link

ProtegeOWLOntologyManager - potential performance problems #354

Closed ykazakov closed 9 years ago

ykazakov commented 9 years ago

I think there may be a potential performance issue in Protege because of this class.

It seems to be a wrapper over OWLOntologyManager which allows to run some tasks for manipulations with OWL ontologies on the event dispatch thread (AWT) of swing. The problem is, if the task takes some time, then the UI freezes. For example, the method applyChanges(...) may trigger listeners for reasoners, which may start reasoning tasks (if the reasoner uses non-buffering mode).

I think that only UI-related things (like drawing, repainting) should be performed on the AWT thread; other (potentially long) computations should be offloaded to other threads. Why wouldn't you use the SwingWorker class instead, which seems to be designed for this purpose?

sesuncedu commented 9 years ago

I've also been looking at moving stuff off the EDT; which will make things not feel as slow. (I happen to have the SwingWorker javadoc :)

I've also been looking at some of the stuff that it's doing that makes it be slow.

One major issue is with variable height lists (or tree) cells; for large ontologies, a huge amount of time is spent manipulating text documents just to calculate heights. With something like the gene ontology and ELK, these calculations can take longer than actual reasoning :)

ignazio1977 commented 9 years ago

I might be wrong but that class is in a Protégé project rather than owl api. Good to know though.

sesuncedu commented 9 years ago

Knowing, yes... learning, not so much. [I have protege, owlapi, and the project I'm actually working on all open in IntelliJ (and sometimes elk)]. It starts to blur :)

It would be great if you could try redoing the protege 5 -> owlapi 4 magic (I wonder how easy the l merge would be?)

ykazakov commented 9 years ago

Hi Ignazio, you are right. It is a part of protege-owlapi. Will move this issue there.

sesuncedu commented 9 years ago

It's really a protegeproject/protege issue, not a protege/protege-owlapi issue (though that is where tat class is)

On Fri, Feb 27, 2015 at 12:58 PM, Yevgeny Kazakov notifications@github.com wrote:

Hi Ignazio, you are right. It is a part of protege-owlapi. Will move this issue there.

— Reply to this email directly or view it on GitHub https://github.com/owlcs/owlapi/issues/354#issuecomment-76440870.

ignazio1977 commented 9 years ago

I'll try again the merge and push :-) time for 4.0.2 anyway.

ykazakov commented 9 years ago

It is a bit strange why protegeproject/protege-owlapi is not a part of protegeproject/protege. Maybe it is also used somewhere else, e.g., webprotege?

matthewhorridge commented 9 years ago

It would be great if you could try redoing the protege 5 -> owlapi 4 magic (I wonder how easy the l merge would be?)

One of the reasons for a bit of feet dragging here is that doing this will currently break Pellet and HermiT (as they aren't compatible with the OWL API 4.0.0) so they won't be available as plugins. I don't see any Pellet or HermiT updates any time soon :(

matthewhorridge commented 9 years ago

One major issue is with variable height lists (or tree) cells; for large ontologies, a huge amount of time is spent manipulating text documents just to calculate heights.

There's some new rendering stuff (see the styled string repo) that was intended to clean up this whole thing.

sesuncedu commented 9 years ago

JIDE-OSS StyledLabel based?

sesuncedu commented 9 years ago

Also, I think that @ignazio1977 did pellet and hermit the first go round?

ignazio1977 commented 9 years ago

I issued a pull request for Pellet and have a HermiT build - HermiT repo is not public so I cannot issue pull requests directly. I'll try and get in touch with the teams responsible to see if there's anything I can do to help things along. Alternatively, we can make independent releases - although I would prefer not to have fragmentation.

matthewhorridge commented 9 years ago

JIDE-OSS StyledLabel based?

It's here.... work in progress!

https://github.com/protegeproject/styledstring