Velocity
  1. Velocity
  2. VELOCITY-797

Inline macros need to be kept with their Template, not centrally managed

    Details

    • Type: Bug Bug
    • Status: Open
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: 1.7
    • Fix Version/s: 2.x
    • Component/s: Engine
    • Labels:
      None

      Description

      This is the proper fix required by VELOCITY-776. Centrally managed inline macros cause race conditions whenever templates are not cached.

      1. VELOCITY-797.patch
        402 kB
        Jarkko Viinamäki

        Issue Links

          Activity

          Hide
          Jarkko Viinamäki added a comment -

          Cleaned up version with JavaDoc fixes etc.

          Show
          Jarkko Viinamäki added a comment - Cleaned up version with JavaDoc fixes etc.
          Hide
          Jarkko Viinamäki added a comment -

          Hi Nathan. Maybe I'll get later back to you about that. I like the fact though that someone reviews big patches so even if I could commit changes straight to SVN, I probably wouldn't do it without review iteration.

          I noticed that I forgot to update some of the JavaDocs and did some cleanup here and there. I'll update the patch soon.

          I also started to run Velocity 2.0 on my performance test bench. I checked out clean version of Velocity 2.0 SVN head (so without any of these changes) and guess what... it is 30%+ slower than Velocity 1.7. I think that's pretty bad so there's definitely a need to do some performance optimization.

          Show
          Jarkko Viinamäki added a comment - Hi Nathan. Maybe I'll get later back to you about that. I like the fact though that someone reviews big patches so even if I could commit changes straight to SVN, I probably wouldn't do it without review iteration. I noticed that I forgot to update some of the JavaDocs and did some cleanup here and there. I'll update the patch soon. I also started to run Velocity 2.0 on my performance test bench. I checked out clean version of Velocity 2.0 SVN head (so without any of these changes) and guess what... it is 30%+ slower than Velocity 1.7. I think that's pretty bad so there's definitely a need to do some performance optimization.
          Hide
          Nathan Bubna added a comment -

          Awesome! Jarkko, any chance you want to accept the offer to be a committer from several years ago? Then you could commit these yourself.

          Show
          Nathan Bubna added a comment - Awesome! Jarkko, any chance you want to accept the offer to be a committer from several years ago? Then you could commit these yourself.
          Hide
          Jarkko Viinamäki added a comment -

          Here's a patch for Velocity 2.0 trunk which implements the change Nathan described i.e. macros are not stored centrally but rather kept within templates. It fixes VELOCITY-776.

          This patch also improves pom.xml to hook JavaCC plugin automatically to the build process, removes a lot of unnecessary code, adds new test cases etc. There are also several internal interface changes (should not affect any Velocity users though).

          All tests pass but these rather radical changes have not been tested otherwise. Careful review is needed.

          Show
          Jarkko Viinamäki added a comment - Here's a patch for Velocity 2.0 trunk which implements the change Nathan described i.e. macros are not stored centrally but rather kept within templates. It fixes VELOCITY-776 . This patch also improves pom.xml to hook JavaCC plugin automatically to the build process, removes a lot of unnecessary code, adds new test cases etc. There are also several internal interface changes (should not affect any Velocity users though). All tests pass but these rather radical changes have not been tested otherwise. Careful review is needed.
          Hide
          Nathan Bubna added a comment -

          This is where an actual fix will happen.

          Show
          Nathan Bubna added a comment - This is where an actual fix will happen.

            People

            • Assignee:
              Unassigned
              Reporter:
              Nathan Bubna
            • Votes:
              1 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:

                Development