Wicket
  1. Wicket
  2. WICKET-4278

Performance regression in Component.configure() in 1.5

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.5.3
    • Fix Version/s: 1.5.4, 6.0.0-beta1
    • Component/s: wicket
    • Labels:
      None

      Description

      Wicket 1.5 is slower than 1.4 in rendering deep component trees.
      See the attached application that adds recursively components with specified depth. In 1.5 it is several times slower than 1.4.

      I found several problems:

      1) org.apache.wicket.Component.configure() calls org.apache.wicket.Component.setRenderAllowed()
      The problems is that configure() is called for each component in the hierarchy and setRenderAllowed() is overwritten in MarkupContainer to visit all children and call it for them. And the final result is that setRenderAllowed() is called for the parent and all its children, then for the first child and all its children again, and so on...

      The solution is to completely remove org.apache.wicket.MarkupContainer.setRenderAllowed()

      2) If you increase the number of components to > 1000 in 1.5 we hit StackOverflowError in
      2.1) org.apache.wicket.MarkupContainer.getMarkupType()
      2.2) org.apache.wicket.Component.getLocale()
      2.3) org.apache.wicket.MarkupContainer.internalMarkRendering(boolean)

      For 2.1 and 2.2 I suggest to add transient fields which will cache the calculated value after the first call of the get method.
      For 2.3 I don't see a solution for now

      1. myproject-1.4.tgz
        5 kB
        Martin Grigorov
      2. myproject-1.5.tgz
        20 kB
        Martin Grigorov
      3. WICKET-4278.patch
        2 kB
        Martin Grigorov
      4. WICKET-4278.patch
        1 kB
        Martin Grigorov

        Activity

        Hide
        Martin Grigorov added a comment -

        The optimization for the locale breaks WicketTester tests but this is completely problem in MockPageManager because it doesn't serialize the pages and the transient field is not cleaned.
        Removing this optimization for now...

        Show
        Martin Grigorov added a comment - The optimization for the locale breaks WicketTester tests but this is completely problem in MockPageManager because it doesn't serialize the pages and the transient field is not cleaned. Removing this optimization for now...
        Hide
        Martin Grigorov added a comment -

        Actually the caching of the locale can be applied. We can null-ify it in Component#onDetach().

        Show
        Martin Grigorov added a comment - Actually the caching of the locale can be applied. We can null-ify it in Component#onDetach().
        Hide
        Sven Meier added a comment -

        setRenderAllowed() was under suspicion already, but I didn't assume that this code could simply be removed .

        Together with caching of locale and markupType we're now down to 15% performance degradation.

        I'll try to profile another hot-spot.

        Show
        Sven Meier added a comment - setRenderAllowed() was under suspicion already, but I didn't assume that this code could simply be removed . Together with caching of locale and markupType we're now down to 15% performance degradation. I'll try to profile another hot-spot.
        Hide
        Sven Meier added a comment -

        As powwowed, I removed MarkupContainer#setRenderAllowed()

        Show
        Sven Meier added a comment - As powwowed, I removed MarkupContainer#setRenderAllowed()

          People

          • Assignee:
            Martin Grigorov
            Reporter:
            Martin Grigorov
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development