On Tue, Oct 30, 2012 at 9:48 AM, Ate Douma <[email protected]> wrote: > Another issue I also experienced but just now properly verified is that > rendering a page (like canonical home page) since the merge has become > excruciating slow. > > I haven't spend any time yet investigating why this is, but I just > compared rendering canonical page 1 with the current trunk against an svn > snapshot of just before the merge. Result: ~ 40 sec. vs. 1 sec. >
I'm not having this problem. The main screen does take 3-4 seconds to load which isn't great but it's not performance like you are seeing. The first time you log into the app after a DB wipe does take a little while but I've always seen that. I think I know what's going on but I need some more time to track it down. For some reason RegionWidgetTag.doStartTag() is being called several hundred times on each page load, which now calls WidgetRepository.get() each time, which is relatively slow (instead of just pulling it from the regionwidget object). Let me debug a little and see what I can do. > > It seems to me something must be completely wrong here and right now the > portal isn't practically usable anymore. > > This is worrisome because I think the model-split is an important step and > amounts to a lot of work by Chris and Matt. I truly hope this is just a > simple configuration issue and nothing fundamental. > Anyone else having a similar experience? > > Note: scheduled for tomorrow is the (start of the) release Rave 0.17, > and next week is ApacheCon EU, where both Matt and myself will have a Rave > presentation and demo. So I'd like to have a reliable release at least by > then. > > Ate > > > On 10/30/2012 05:11 PM, Ate Douma wrote: > >> On 10/29/2012 05:11 AM, Chris Geer wrote: >> >>> Branch is now merged into trunk. The only change I had to make was to >>> the OmdlOutputAdapter class where I had to comment out the @Component >>> annotation. This was because that class now needed a constructor that >>> took >>> several services to function and @Autowire wasn't working due to order of >>> creation issues I believe. The only place I could find it was used in the >>> code was a place it was manually constructed so I think we are fine but I >>> may have missed something. I ran all the unit tests, integration tests >>> and >>> also spot checked a lot of the capabilities. >>> >> >> Hi Chris, >> >> While trying to reconcile these model-split changes against trunk with the >> content-services sandbox, I encountered a few issues, all related to the >> RegionWidget - Widget separation. >> One minor issue is I think a simple merge error but others aspects are >> more >> structural. >> >> First of all, currently Rave breaks when accessed from a mobile device: >> mobile >> rendering now throws a JSP rendering exception because the >> <rave:render-widget/> >> tag has an extra and required widget parameter which hasn't been set >> (mobile.jsp >> #78). >> >> Another issue is that the Profile page no longer renders (static) widgets >> because the ProfileController doesn't derive and saves the list of >> widgets in >> the page to the model (as PageController does). I think you did have this >> in the >> branch but it got lost during the merge back into trunk. >> >> This functionality, retrieving and deriving the widgets from the page >> region(s) >> RegionWidgets and storing them as a list on the model so they are >> available >> during rendering, I'm not very happy with. Especially not when it is >> stored as a >> list. A map (WidgetId->Widget) might be better already so we don't need to >> iterate on the whole list to find back a matching single widget, as for >> example >> now done in region.tag. >> But maybe an even cleaner alternative would be retrieving a Widget 'on >> demand' >> through a custom tag itself, similar to the PersonTag does? >> And tags as RegionWidgetTag then can do this internally and then wouldn't >> need >> the extra widget parameter. >> >> Thoughts? >> >> >>> After integration I did find one bug >>> RAVE-837<https://issues.**apache.org/jira/browse/RAVE-**837<https://issues.apache.org/jira/browse/RAVE-837>> >>> that >>> I will try and fix in the next day or so. Please file tickets for any >>> other >>> bugs and we'll squash them. >>> >>> Just remember now, all the IDs in the interfaces are Strings now, not >>> Longs. Longs are still used in the JPA implementation and converted to >>> strings at the interface level. >>> >>> @Jasha - I fixed the category problem prior to merge. >>> >>> Chris >>> >>> On Fri, Oct 26, 2012 at 4:30 AM, Jasha Joachimsthal <[email protected] >>> >wrote: >>> >>> Hi Chris, >>>> >>>> I did a few tests and found the following things: >>>> >>>> Widget Store shows a log entry about tags: >>>> [INFO] [talledLocalContainer] 465793 ravePersistenceUnit WARN >>>> [http-8080-1] openjpa.Runtime - Supplied user parameters "[widgetId, >>>> tagId]" do not match expected parameters "[widgetId]" for the prepared >>>> query "PreparedQuery: [select t from JpaWidgetTag t where >>>> t.widgetId=:widgetId and t.tagId=:tagId] --> [SELECT t0.entity_id, >>>> t0.created_date, t0.tag_id, t0.user_id, t0.widget_id FROM widget_tag t0 >>>> WHERE (t0.widget_id = ? AND t0.tag_id IS NULL)]". >>>> >>>> In the Widget store if you select a category and then select the empty >>>> category you get an error page: >>>> URL: >>>> >>>> http://localhost:8080/portal/**app/store/category?categoryId=** >>>> 0&referringPageId=1(logged<http://localhost:8080/portal/app/store/category?categoryId=0&referringPageId=1(logged> >>>> >>>> in as canonical) >>>> [WARNING] [talledLocalContainer] java.lang.NullPointerException >>>> [WARNING] [talledLocalContainer] at >>>> >>>> org.apache.rave.portal.**service.impl.**DefaultWidgetService.** >>>> getWidgetsByCategory(**DefaultWidgetService.java:181) >>>> >>>> >>>> The integration tests passed except the last one (OpenID login), but >>>> that >>>> one has been fixed in the trunk. >>>> No real blockers IMO. >>>> >>>> Jasha >>>> >>>> >>>> On 25 October 2012 19:12, Chris Geer <[email protected]> wrote: >>>> >>>> All, I will be merging this over the weekend if there are no >>>>> objections. >>>>> (Sunday should be 72 hours). At that time I will do the final reverse >>>>> >>>> merge >>>> >>>>> into the branch and merge into trunk. >>>>> >>>>> Chris >>>>> >>>>> On Fri, Oct 12, 2012 at 5:29 PM, Chris Geer <[email protected]> >>>>> >>>> wrote: >>>> >>>>> >>>>> I think we are at a point where the model-split work is far enough >>>>>> >>>>> along >>>> >>>>> that we should consider merging it soon. I need to finish up >>>>>> >>>>> refactoring >>>> >>>>> WidgetRating but after that there will be enough critical mass to merge >>>>>> >>>>> it >>>>> >>>>>> into the baseline IMHO. Prior to that happening it would be good if we >>>>>> >>>>> can >>>>> >>>>>> get several people to spend a little time and review the changes that >>>>>> >>>>> have >>>>> >>>>>> been made. Any volunteers? >>>>>> >>>>>> The major changes are: >>>>>> 1) All IDs in the interfaces have been changed from int to String. >>>>>> This >>>>>> >>>>> is >>>>> >>>>>> to support a more flexible ID structure in the future with backends >>>>>> >>>>> other >>>> >>>>> than JPA >>>>>> 2) The model has been split into several related chunks. The major >>>>>> >>>>> chunks >>>> >>>>> are Users/People, Widget and other right now. Object relationships >>>>>> >>>>> between >>>>> >>>>>> those groups have been broken and replaced with IDs. This is to >>>>>> >>>>> support a >>>> >>>>> more modular backend eventually. Right now they are all still part of >>>>>> >>>>> the >>>> >>>>> same JPA persistance unit but that is short term. >>>>>> 3) The widget model has been changed to make Widget the top level >>>>>> >>>>> object >>>> >>>>> and WidgetComment, WidgetTag & WidgetRating are subordinate objects. >>>>>> >>>>> These >>>>> >>>>>> changes include removing the widgetID attribute from the subordinate >>>>>> >>>>> object >>>>> >>>>>> so that they are associated with the Widget they are attached to. We >>>>>> >>>>> also >>>> >>>>> consolidated the various services and repositories into the >>>>>> WidgetService/Repository since acting on the subordinate >>>>>> object independent of the widget isn't ideal. >>>>>> 4) The link between RegionWidget and Widget has been replaced with >>>>>> WidgetID. This has required that we populate the JSP attributes with >>>>>> >>>>> the >>>> >>>>> list of widgets on a page (in the controller). >>>>>> >>>>>> There are plenty of other changes we could make so if there are any >>>>>> you >>>>>> think are crucial before the merge please point those out but I'm >>>>>> >>>>> hoping >>>> >>>>> we >>>>> >>>>>> can get the major changes into trunk and then iterate through the >>>>>> rest. >>>>>> >>>>>> Thanks, >>>>>> Chris >>>>>> >>>>>> >>>>> >>>> >>> >> >
