Loading ...
Sorry, an error occurred while loading the content.

Re: [pcgen_developers] gamemode/campaign filter

Expand Messages
  • Chris Dolan
    Thanks very much, James. I ve implemented all of your suggestions: https://github.com/chrisdolan/pcgen-svn/commit/8036dab4060a53eb52016b016b779839aa5debed
    Message 1 of 8 , Jan 30, 2013
    • 0 Attachment
      Thanks very much, James. I've implemented all of your suggestions:
      https://github.com/chrisdolan/pcgen-svn/commit/8036dab4060a53eb52016b016b779839aa5debed
      Chris

      On Jan 30, 2013, at 1:25 AM, James Dempsey wrote:

      > Hi Chris,
      >
      > Great to see you are getting good progress here!
      >
      > While the main PCGen code would not use that code, it looks safe to
      > incorporate into the code base, which could be convenient for you and
      > anyone else tackling mobile or small footprint work.
      >
      > With that in mind, I noticed a few things as I read through the patch:
      >
      > * Field definitions should be at the top of the file (its a bit
      > confuising as there was already a block of fields in the guts of the
      > CampaignFileLoader file).
      > * Single line if blocks should be surrounded by braces (see our coding
      > standards at http://wiki.pcgen.org/Coding_Standards )
      > * Great to see new File being used to make up paths rather than string
      > concatenation :)
      > * In CampaignLoader, be careful of method parameters obscuring fields
      > (e.g. campaign) - it works for simple setters or constructors but is a
      > lurking trap in other code.
      > * Comments for each method called from your product would help future
      > developers know what not to cut out as it is used externally.
      >
      > Cheers,
      > James.
      >
      >
      > On 30/01/2013 5:49 PM Chris Dolan wrote
      >> I made a new commit tonight that saved 7MB of heap in my benchmark (that is, about a 10% improvement). Here's the diff:
      >> https://github.com/chrisdolan/pcgen-svn/commit/eb274a4779c1d6157a43e46b9972e8d2493aa3cc
      >>
      >> This change adds optional filters to the GameModeFileLoader and the CampaignFileLoader. The filter is a callback to user code, which can reject irrelevant data files. In my case, I do a quick-and-dirty parse of the .pcg file to get the CAMPAIGN and GAMEMODE lines, and reject all that don't match. The campaign loader already had some fix-up code that recursively loads other campaigns that the main one depends on.
      >>
      >> Not surprisingly, the majority of that 7MB savings came from the campaign filter. The improvement from the game mode filter was tiny by contrast. Because the GameModeFileLoader messes with Globals more than the CampaignFileLoader, I'd recommend that other people do not use my game mode filter unless you're *sure* that all loaded PCs will be the same game mode.
      >>
      >>
      >>
      >> My next benchmark will be Tom's idea to lazy load the HashMaps in the CDOM. Eclipse MAT predicts that I can save up to 13MB(!) by such techniques. The following table counts all of the HashMap instances in heap and sorts them by how many elements they contain. As you can see there are over 100,000 empty hashmaps. I don't yet know what fraction of those are in CDOM instances.
      >>
      >> Length # Objects Shallow Heap Retained Heap
      >> 0 106,792 5,126,016 13,682,144
      >> 1 22,808 1,094,784 7,786,272
      >> 2 13,684 656,832 7,877,944
      >> 3 14,614 701,472 6,757,224
      >> 4 4,514 216,672 2,627,064
      >> 5 9,159 439,632 3,685,112
      >> 6 3,184 152,832 1,714,720
      >> ...
      >>
      >> A related savings could come via tweaks to the fill ratio for the hashmaps. We could optimize for space instead of speed in some cases.
      >>
      >> Chris
      >>
      >
      >
      >
      > ------------------------------------
      >
      > Yahoo! Groups Links
      >
      >
      >
    • Chris Dolan
      Woohoo!!! 12 MB reclaimed! I m down to 55 MB of heap in my benchmark. Here s the same table as below, now reduced by 90,000 empty HashMaps via the
      Message 2 of 8 , Jan 30, 2013
      • 0 Attachment
        Woohoo!!! 12 MB reclaimed! I'm down to 55 MB of heap in my benchmark.

        Here's the same table as below, now reduced by 90,000 empty HashMaps via the lazy-initialization refactor of CDOMObject:

        Length # Objects Shallow Heap Retained Heap
        0 10,590 508,320 1,318,576
        1 22,808 1,094,784 7,691,984
        2 13,684 656,832 7,340,056
        3 14,614 701,472 6,596,336
        4 4,514 216,672 2,566,472
        5 9,159 439,632 3,600,488
        6 3,184 152,832 1,661,672

        The gory details (pretty straightforward, actually, but lots of little tweaks)
        THANK YOU for the excellent unit tests! I made a few bugs in my refactor initially, but the tests caught them.
        I noted a few places where the JavaDoc doesn't agree with the implementation. It would be useful if someone would review those notes and decide which is right.

        Chris


        On Jan 30, 2013, at 9:11 AM, Tom Parker wrote:



        Nice data on the HashMaps.  Didn't realize it would be so much. :)

        TP.
        --
        Tom Parker


        From: Chris Dolan <chris@...>
        To: pcgen_developers@yahoogroups.com 
        Sent: Wednesday, January 30, 2013 1:49 AM
        Subject: [pcgen_developers] gamemode/campaign filter

        I made a new commit tonight that saved 7MB of heap in my benchmark (that is, about a 10% improvement). Here's the diff:
          https://github.com/chrisdolan/pcgen-svn/commit/eb274a4779c1d6157a43e46b9972e8d2493aa3cc

        This change adds optional filters to the GameModeFileLoader and the CampaignFileLoader. The filter is a callback to user code, which can reject irrelevant data files. In my case, I do a quick-and-dirty parse of the .pcg file to get the CAMPAIGN and GAMEMODE lines, and reject all that don't match. The campaign loader already had some fix-up code that recursively loads other campaigns that the main one depends on.

        Not surprisingly, the majority of that 7MB savings came from the campaign filter. The improvement from the game mode filter was tiny by contrast. Because the GameModeFileLoader messes with Globals more than the CampaignFileLoader, I'd recommend that other people do not use my game mode filter unless you're *sure* that all loaded PCs will be the same game mode.



        My next benchmark will be Tom's idea to lazy load the HashMaps in the CDOM. Eclipse MAT predicts that I can save up to 13MB(!) by such techniques. The following table counts all of the HashMap instances in heap and sorts them by how many elements they contain. As you can see there are over 100,000 empty hashmaps. I don't yet know what fraction of those are in CDOM instances.

        Length    # Objects    Shallow Heap    Retained Heap
        0    106,792        5,126,016    13,682,144
        1    22,808        1,094,784    7,786,272
        2    13,684        656,832        7,877,944
        3    14,614        701,472        6,757,224
        4    4,514        216,672        2,627,064
        5    9,159        439,632        3,685,112
        6    3,184        152,832        1,714,720
        ...

        A related savings could come via tweaks to the fill ratio for the hashmaps. We could optimize for space instead of speed in some cases.

        Chris




      • Tom Parker
        Nice. ... and thanks for the heads up on the JavaDocs.  I ll take a look.  A bunch of it is my code so I can review it pretty quickly. TP. p.s. Similar
        Message 3 of 8 , Jan 31, 2013
        • 0 Attachment
          Nice.

          ... and thanks for the heads up on the JavaDocs.  I'll take a look.  A bunch of it is my code so I can review it pretty quickly.

          TP.

          p.s. Similar comments here to James' earlier comments - this still has one line IFs with no braces.   Once that is cleaned up, we should probably pull this into the trunk.  The reduced memory and thus reduced memory allocations during load is probably a material benefit.
          --
          Tom Parker


          From: Chris Dolan <chris@...>
          To: pcgen_developers@yahoogroups.com
          Sent: Thursday, January 31, 2013 2:05 AM
          Subject: Re: [pcgen_developers] gamemode/campaign filter



          Woohoo!!! 12 MB reclaimed! I'm down to 55 MB of heap in my benchmark.

          Here's the same table as below, now reduced by 90,000 empty HashMaps via the lazy-initialization refactor of CDOMObject:

          Length # Objects Shallow Heap Retained Heap
          0 10,590 508,320 1,318,576
          1 22,808 1,094,784 7,691,984
          2 13,684 656,832 7,340,056
          3 14,614 701,472 6,596,336
          4 4,514 216,672 2,566,472
          5 9,159 439,632 3,600,488
          6 3,184 152,832 1,661,672

          The gory details (pretty straightforward, actually, but lots of little tweaks)
          THANK YOU for the excellent unit tests! I made a few bugs in my refactor initially, but the tests caught them.
          I noted a few places where the JavaDoc doesn't agree with the implementation. It would be useful if someone would review those notes and decide which is right.

          Chris


          On Jan 30, 2013, at 9:11 AM, Tom Parker wrote:



          Nice data on the HashMaps.  Didn't realize it would be so much. :)

          TP.
          --
          Tom Parker


          From: Chris Dolan <chris@...>
          To: pcgen_developers@yahoogroups.com 
          Sent: Wednesday, January 30, 2013 1:49 AM
          Subject: [pcgen_developers] gamemode/campaign filter

          I made a new commit tonight that saved 7MB of heap in my benchmark (that is, about a 10% improvement). Here's the diff:
            https://github.com/chrisdolan/pcgen-svn/commit/eb274a4779c1d6157a43e46b9972e8d2493aa3cc

          This change adds optional filters to the GameModeFileLoader and the CampaignFileLoader. The filter is a callback to user code, which can reject irrelevant data files. In my case, I do a quick-and-dirty parse of the .pcg file to get the CAMPAIGN and GAMEMODE lines, and reject all that don't match. The campaign loader already had some fix-up code that recursively loads other campaigns that the main one depends on.

          Not surprisingly, the majority of that 7MB savings came from the campaign filter. The improvement from the game mode filter was tiny by contrast. Because the GameModeFileLoader messes with Globals more than the CampaignFileLoader, I'd recommend that other people do not use my game mode filter unless you're *sure* that all loaded PCs will be the same game mode.



          My next benchmark will be Tom's idea to lazy load the HashMaps in the CDOM. Eclipse MAT predicts that I can save up to 13MB(!) by such techniques. The following table counts all of the HashMap instances in heap and sorts them by how many elements they contain. As you can see there are over 100,000 empty hashmaps. I don't yet know what fraction of those are in CDOM instances.

          Length    # Objects    Shallow Heap    Retained Heap
          0    106,792        5,126,016    13,682,144
          1    22,808        1,094,784    7,786,272
          2    13,684        656,832        7,877,944
          3    14,614        701,472        6,757,224
          4    4,514        216,672        2,627,064
          5    9,159        439,632        3,685,112
          6    3,184        152,832        1,714,720
          ...

          A related savings could come via tweaks to the fill ratio for the hashmaps. We could optimize for space instead of speed in some cases.

          Chris








        • Chris Dolan
          Fixed. The extra curlies is a style that I find difficult to remember... Have you folks considered adding a .settings folder with Eclipse project-specific
          Message 4 of 8 , Jan 31, 2013
          • 0 Attachment
            Fixed. The extra curlies is a style that I find difficult to remember... Have you folks considered adding a .settings folder with Eclipse project-specific settings to help enforce those rules?

            Chris


            On Jan 31, 2013, at 9:15 AM, Tom Parker wrote:



            Nice.

            ... and thanks for the heads up on the JavaDocs.  I'll take a look.  A bunch of it is my code so I can review it pretty quickly.

            TP.

            p.s. Similar comments here to James' earlier comments - this still has one line IFs with no braces.   Once that is cleaned up, we should probably pull this into the trunk.  The reduced memory and thus reduced memory allocations during load is probably a material benefit.
            --
            Tom Parker


            From: Chris Dolan <chris@...>
            To: pcgen_developers@yahoogroups.com 
            Sent: Thursday, January 31, 2013 2:05 AM
            Subject: Re: [pcgen_developers] gamemode/campaign filter



            Woohoo!!! 12 MB reclaimed! I'm down to 55 MB of heap in my benchmark.

            Here's the same table as below, now reduced by 90,000 empty HashMaps via the lazy-initialization refactor of CDOMObject:

            Length # Objects Shallow Heap Retained Heap
            0 10,590 508,320 1,318,576
            1 22,808 1,094,784 7,691,984
            2 13,684 656,832 7,340,056
            3 14,614 701,472 6,596,336
            4 4,514 216,672 2,566,472
            5 9,159 439,632 3,600,488
            6 3,184 152,832 1,661,672

            The gory details (pretty straightforward, actually, but lots of little tweaks)
            THANK YOU for the excellent unit tests! I made a few bugs in my refactor initially, but the tests caught them.
            I noted a few places where the JavaDoc doesn't agree with the implementation. It would be useful if someone would review those notes and decide which is right.

            Chris


            On Jan 30, 2013, at 9:11 AM, Tom Parker wrote:



            Nice data on the HashMaps.  Didn't realize it would be so much. :)

            TP.
            --
            Tom Parker


            From: Chris Dolan <chris@...>
            To: pcgen_developers@yahoogroups.com 
            Sent: Wednesday, January 30, 2013 1:49 AM
            Subject: [pcgen_developers] gamemode/campaign filter

            I made a new commit tonight that saved 7MB of heap in my benchmark (that is, about a 10% improvement). Here's the diff:
              https://github.com/chrisdolan/pcgen-svn/commit/eb274a4779c1d6157a43e46b9972e8d2493aa3cc

            This change adds optional filters to the GameModeFileLoader and the CampaignFileLoader. The filter is a callback to user code, which can reject irrelevant data files. In my case, I do a quick-and-dirty parse of the .pcg file to get the CAMPAIGN and GAMEMODE lines, and reject all that don't match. The campaign loader already had some fix-up code that recursively loads other campaigns that the main one depends on.

            Not surprisingly, the majority of that 7MB savings came from the campaign filter. The improvement from the game mode filter was tiny by contrast. Because the GameModeFileLoader messes with Globals more than the CampaignFileLoader, I'd recommend that other people do not use my game mode filter unless you're *sure* that all loaded PCs will be the same game mode.



            My next benchmark will be Tom's idea to lazy load the HashMaps in the CDOM. Eclipse MAT predicts that I can save up to 13MB(!) by such techniques. The following table counts all of the HashMap instances in heap and sorts them by how many elements they contain. As you can see there are over 100,000 empty hashmaps. I don't yet know what fraction of those are in CDOM instances.

            Length    # Objects    Shallow Heap    Retained Heap
            0    106,792        5,126,016    13,682,144
            1    22,808        1,094,784    7,786,272
            2    13,684        656,832        7,877,944
            3    14,614        701,472        6,757,224
            4    4,514        216,672        2,627,064
            5    9,159        439,632        3,685,112
            6    3,184        152,832        1,714,720
            ...

            A related savings could come via tweaks to the fill ratio for the hashmaps. We could optimize for space instead of speed in some cases.

            Chris











          • Tom Parker
            There is an eclipse settings file in pcgen/code/settings that gives you the formatting rules, et al. TP. -- Tom Parker ________________________________ From:
            Message 5 of 8 , Jan 31, 2013
            • 0 Attachment
              There is an eclipse settings file in pcgen/code/settings that gives you the formatting rules, et al.

              TP.
              --
              Tom Parker


              From: Chris Dolan <chris@...>
              To: pcgen_developers@yahoogroups.com
              Sent: Thursday, January 31, 2013 9:14 PM
              Subject: Re: [pcgen_developers] gamemode/campaign filter



              Fixed. The extra curlies is a style that I find difficult to remember... Have you folks considered adding a .settings folder with Eclipse project-specific settings to help enforce those rules?

              Chris


              On Jan 31, 2013, at 9:15 AM, Tom Parker wrote:



              Nice.

              ... and thanks for the heads up on the JavaDocs.  I'll take a look.  A bunch of it is my code so I can review it pretty quickly.

              TP.

              p.s. Similar comments here to James' earlier comments - this still has one line IFs with no braces.   Once that is cleaned up, we should probably pull this into the trunk.  The reduced memory and thus reduced memory allocations during load is probably a material benefit.
              --
              Tom Parker


              From: Chris Dolan <chris@...>
              To: pcgen_developers@yahoogroups.com 
              Sent: Thursday, January 31, 2013 2:05 AM
              Subject: Re: [pcgen_developers] gamemode/campaign filter



              Woohoo!!! 12 MB reclaimed! I'm down to 55 MB of heap in my benchmark.

              Here's the same table as below, now reduced by 90,000 empty HashMaps via the lazy-initialization refactor of CDOMObject:

              Length # Objects Shallow Heap Retained Heap
              0 10,590 508,320 1,318,576
              1 22,808 1,094,784 7,691,984
              2 13,684 656,832 7,340,056
              3 14,614 701,472 6,596,336
              4 4,514 216,672 2,566,472
              5 9,159 439,632 3,600,488
              6 3,184 152,832 1,661,672

              The gory details (pretty straightforward, actually, but lots of little tweaks)
              THANK YOU for the excellent unit tests! I made a few bugs in my refactor initially, but the tests caught them.
              I noted a few places where the JavaDoc doesn't agree with the implementation. It would be useful if someone would review those notes and decide which is right.

              Chris


              On Jan 30, 2013, at 9:11 AM, Tom Parker wrote:



              Nice data on the HashMaps.  Didn't realize it would be so much. :)

              TP.
              --
              Tom Parker


              From: Chris Dolan <chris@...>
              To: pcgen_developers@yahoogroups.com 
              Sent: Wednesday, January 30, 2013 1:49 AM
              Subject: [pcgen_developers] gamemode/campaign filter

              I made a new commit tonight that saved 7MB of heap in my benchmark (that is, about a 10% improvement). Here's the diff:
                https://github.com/chrisdolan/pcgen-svn/commit/eb274a4779c1d6157a43e46b9972e8d2493aa3cc

              This change adds optional filters to the GameModeFileLoader and the CampaignFileLoader. The filter is a callback to user code, which can reject irrelevant data files. In my case, I do a quick-and-dirty parse of the .pcg file to get the CAMPAIGN and GAMEMODE lines, and reject all that don't match. The campaign loader already had some fix-up code that recursively loads other campaigns that the main one depends on.

              Not surprisingly, the majority of that 7MB savings came from the campaign filter. The improvement from the game mode filter was tiny by contrast. Because the GameModeFileLoader messes with Globals more than the CampaignFileLoader, I'd recommend that other people do not use my game mode filter unless you're *sure* that all loaded PCs will be the same game mode.



              My next benchmark will be Tom's idea to lazy load the HashMaps in the CDOM. Eclipse MAT predicts that I can save up to 13MB(!) by such techniques. The following table counts all of the HashMap instances in heap and sorts them by how many elements they contain. As you can see there are over 100,000 empty hashmaps. I don't yet know what fraction of those are in CDOM instances.

              Length    # Objects    Shallow Heap    Retained Heap
              0    106,792        5,126,016    13,682,144
              1    22,808        1,094,784    7,786,272
              2    13,684        656,832        7,877,944
              3    14,614        701,472        6,757,224
              4    4,514        216,672        2,627,064
              5    9,159        439,632        3,685,112
              6    3,184        152,832        1,714,720
              ...

              A related savings could come via tweaks to the fill ratio for the hashmaps. We could optimize for space instead of speed in some cases.

              Chris















            Your message has been successfully submitted and would be delivered to recipients shortly.