Details

      Description

      There are some egregious resource leaks from two of the NSS test suites that you developed. I just tried to close those leaks, but the test structure is a bit complex and my changes are not working. I suspect that setUp() and tearDown() might not be called in a manner that consistent with my expectations.

      The basic problem is that ALL instance variables in a unit test MUST be set to null in tearDown. If they are not set to null, then the will live until the test suite completes. Since we have 18000 tests, that turns into a big problem in CI. Some of the potentially leaked objects are piggish, including HttpClient objects and even Servlet objects.

      The culprits are:
      - AbstractProtocolTest
      - AbstractNamedGraphUpdateTest

      These are used from the following concrete test suites:
      - ExampleProtocolTest
      - TestAskJsonTrac704
      - TestPostNotURLEncoded
      - TestRelease123Protocol
      - TestService794

      I have attached the versions where I tried to fix this.

      I would appreciate it if you could take a look at this ASAP. Otherwise I will need to remove these tests from CI until this is resolved. Right now it is blocking me from getting a clean NSS test suite run.

      1. after.jpg
        436 kB
      2. before.jpg
        478 kB

        Activity

        Hide
        jeremycarroll jeremycarroll added a comment -

        I am hopeful that r8657 resolves this. In my test scenario (10,000 instances of ExampleProtocolTest) I observed a 90% reduction in memory leakage. While this is not perfect, I am unclear how much memory a completed test should be using (to record the test result, etc.) and I am hopeful that the improvement is good enough to unblock the CI

        Show
        jeremycarroll jeremycarroll added a comment - I am hopeful that r8657 resolves this. In my test scenario (10,000 instances of ExampleProtocolTest) I observed a 90% reduction in memory leakage. While this is not perfect, I am unclear how much memory a completed test should be using (to record the test result, etc.) and I am hopeful that the improvement is good enough to unblock the CI
        Hide
        jeremycarroll jeremycarroll added a comment -

        Attachment before.jpg has been added with description: your_kit showing before leakage of 150k/s midstream

        Show
        jeremycarroll jeremycarroll added a comment - Attachment before.jpg has been added with description: your_kit showing before leakage of 150k/s midstream
        Hide
        jeremycarroll jeremycarroll added a comment -

        Attachment after.jpg has been added with description: yourkit snapshot showing approx 15k/s leakage midstream

        Show
        jeremycarroll jeremycarroll added a comment - Attachment after.jpg has been added with description: yourkit snapshot showing approx 15k/s leakage midstream
        Hide
        bryanthompson bryanthompson added a comment -

        Still a blocker. Try w/ a 2G heap. Will fail in the test suites indicated above.

        Show
        bryanthompson bryanthompson added a comment - Still a blocker. Try w/ a 2G heap. Will fail in the test suites indicated above.
        Hide
        bryanthompson bryanthompson added a comment -

        I am able to get through a run of the NSS test suite now on a 2G heap, so things have definitely improved.

        Show
        bryanthompson bryanthompson added a comment - I am able to get through a run of the NSS test suite now on a 2G heap, so things have definitely improved.
        Hide
        jeremycarroll jeremycarroll added a comment -

        I would like to suggest closing this then; I do not believe the situation is perfect ?. but I suspect it is good enough and is no longer top-of-the-pile. We should probably wait for the current build to finish, I backed out some remnants of the ill-conceived clearStandAloneQECacheDuringTesting.

        Show
        jeremycarroll jeremycarroll added a comment - I would like to suggest closing this then; I do not believe the situation is perfect ?. but I suspect it is good enough and is no longer top-of-the-pile. We should probably wait for the current build to finish, I backed out some remnants of the ill-conceived clearStandAloneQECacheDuringTesting.
        Hide
        bryanthompson bryanthompson added a comment -

        I am fine with that as long as the next build goes smoothly.

        Show
        bryanthompson bryanthompson added a comment - I am fine with that as long as the next build goes smoothly.

          People

          • Assignee:
            jeremycarroll jeremycarroll
            Reporter:
            bryanthompson bryanthompson
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: