Uploaded image for project: 'Lucene - Core'
  1. Lucene - Core
  2. LUCENE-7595

RAMUsageTester in test-framework and static field checker no longer works with Java 9

    Details

    • Lucene Fields:
      New

      Description

      Lucene/Solr tests have a special rule that records memory usage in static fields before and after test, so we can detect memory leaks. This check dives into JDK classes (like java.lang.String to detect their size). As Java 9 build 148 completely forbids setAccessible on any runtime class, we have to change or disable this check:

      • As first step I will only add the rule to LTC, if we not have Java 8
      • As a second step we might investigate how to improve this

      Robert Muir had some ideas for the 2nd point:

      • Don't dive into classes from JDK modules and instead "estimate" the size for some special cases (like Strings)
      • Disallow any static field in tests that is not final (constant) and points to an Object except: Strings and native (wrapper) types.

      In addition we also have RAMUsageTester, that has similar problems and is used to compare estimations of Lucene's calculations of Codec/IndexWriter/IndexReader memory usage with reality. We should simply disable those tests.

      1. LUCENE-7595.patch
        13 kB
        Uwe Schindler

        Issue Links

          Activity

          Hide
          dweiss Dawid Weiss added a comment -

          I agree certain classes could be approximated (like String, Lists, etc.).

          Disallow any static field in tests that is not final (constant) and points to an Object except: Strings and native (wrapper) types.

          The check could be less strict – we could fail if the value of such a field is non-null after the test and permit nullified reference fields.

          Show
          dweiss Dawid Weiss added a comment - I agree certain classes could be approximated (like String, Lists, etc.). Disallow any static field in tests that is not final (constant) and points to an Object except: Strings and native (wrapper) types. The check could be less strict – we could fail if the value of such a field is non-null after the test and permit nullified reference fields.
          Hide
          thetaphi Uwe Schindler added a comment - - edited

          Here is my patch that makes test works on whole Lucene:

          • On Java 9 it disables the static leak detector
          • RamUsageTester was fixed to have some "shortcuts" which are used if Java 9+ is detected: String/StringBuffer/StringBuilder and some other types are calculated using their length/capacity. It also estimates memory usage of Maps and Iterables by just iterating over their items (not respecting the Hash/LinkedHash impl details, just plain stupid summing up). Because of this I had to disable one test for the LRU cache, but otherwise the estimation is almost correct. All other uses of RamUsageTester pass

          Dawid Weiss: What do you think?

          Show
          thetaphi Uwe Schindler added a comment - - edited Here is my patch that makes test works on whole Lucene: On Java 9 it disables the static leak detector RamUsageTester was fixed to have some "shortcuts" which are used if Java 9+ is detected: String/StringBuffer/StringBuilder and some other types are calculated using their length/capacity. It also estimates memory usage of Maps and Iterables by just iterating over their items (not respecting the Hash/LinkedHash impl details, just plain stupid summing up). Because of this I had to disable one test for the LRU cache, but otherwise the estimation is almost correct. All other uses of RamUsageTester pass Dawid Weiss : What do you think?
          Hide
          dweiss Dawid Weiss added a comment -

          Looks good to me overall. A few notes and ideas.

          There is a separate RUE impl. inside randomized runner just for the sake of StaticFieldsInvariantRule... perhaps we could simply copy over this rule's source code to Lucene to make use of the local ram estimator – then you'd have full control to tweak it as needed. Perhaps it'd also simplify the static rule initialization block.

          +      a(String.class, v -> charArraySize(v.length())); // may not be correct with Java 9's compact strings!
          

          Yup, this is overestimating based on the patch (https://bugs.openjdk.java.net/browse/JDK-8054307). But I think it's ok – better to overestimate here and blow up early and there's no way to check which representation (byte or char-based) was actually chosen by the jvm. Yes, we could reimplement the Latin1 vs. UTF16 heuristic, but it seems like an overkill.

          Show
          dweiss Dawid Weiss added a comment - Looks good to me overall. A few notes and ideas. There is a separate RUE impl. inside randomized runner just for the sake of StaticFieldsInvariantRule ... perhaps we could simply copy over this rule's source code to Lucene to make use of the local ram estimator – then you'd have full control to tweak it as needed. Perhaps it'd also simplify the static rule initialization block. + a( String .class, v -> charArraySize(v.length())); // may not be correct with Java 9's compact strings! Yup, this is overestimating based on the patch ( https://bugs.openjdk.java.net/browse/JDK-8054307 ). But I think it's ok – better to overestimate here and blow up early and there's no way to check which representation (byte or char-based) was actually chosen by the jvm. Yes, we could reimplement the Latin1 vs. UTF16 heuristic, but it seems like an overkill.
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit f29d2b5668296dfcdb8d650305449674faa29847 in lucene-solr's branch refs/heads/master from Uwe Schindler
          [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=f29d2b5 ]

          LUCENE-7595: Improve RAMUsageTester in test-framework to estimate memory usage of runtime classes and work with Java 9 EA (b148+). Disable static field heap usage checker in LuceneTestCase

          Show
          jira-bot ASF subversion and git services added a comment - Commit f29d2b5668296dfcdb8d650305449674faa29847 in lucene-solr's branch refs/heads/master from Uwe Schindler [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=f29d2b5 ] LUCENE-7595 : Improve RAMUsageTester in test-framework to estimate memory usage of runtime classes and work with Java 9 EA (b148+). Disable static field heap usage checker in LuceneTestCase
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit ea7db0a176979559e874b292522fa7006b578882 in lucene-solr's branch refs/heads/branch_6x from Uwe Schindler
          [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=ea7db0a ]

          LUCENE-7595: Improve RAMUsageTester in test-framework to estimate memory usage of runtime classes and work with Java 9 EA (b148+). Disable static field heap usage checker in LuceneTestCase

          Show
          jira-bot ASF subversion and git services added a comment - Commit ea7db0a176979559e874b292522fa7006b578882 in lucene-solr's branch refs/heads/branch_6x from Uwe Schindler [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=ea7db0a ] LUCENE-7595 : Improve RAMUsageTester in test-framework to estimate memory usage of runtime classes and work with Java 9 EA (b148+). Disable static field heap usage checker in LuceneTestCase
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit db9190db9372ae88a7392a7186397441ce070a96 in lucene-solr's branch refs/heads/master from Uwe Schindler
          [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=db9190d ]

          LUCENE-7595: Fix bug with RamUsageTester incorrectly handling Iterables outside Java Runtime

          Show
          jira-bot ASF subversion and git services added a comment - Commit db9190db9372ae88a7392a7186397441ce070a96 in lucene-solr's branch refs/heads/master from Uwe Schindler [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=db9190d ] LUCENE-7595 : Fix bug with RamUsageTester incorrectly handling Iterables outside Java Runtime
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 80512ec412c20517341ddd50c78baf5270fcdc2f in lucene-solr's branch refs/heads/branch_6x from Uwe Schindler
          [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=80512ec ]

          LUCENE-7595: Fix bug with RamUsageTester incorrectly handling Iterables outside Java Runtime

          Show
          jira-bot ASF subversion and git services added a comment - Commit 80512ec412c20517341ddd50c78baf5270fcdc2f in lucene-solr's branch refs/heads/branch_6x from Uwe Schindler [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=80512ec ] LUCENE-7595 : Fix bug with RamUsageTester incorrectly handling Iterables outside Java Runtime
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit d65c02e8cc14f03389c2426ea3d3ddd75e12b1ec in lucene-solr's branch refs/heads/master from Uwe Schindler
          [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=d65c02e ]

          LUCENE-7595: Disable another test not compatible with RamUsageTester

          Show
          jira-bot ASF subversion and git services added a comment - Commit d65c02e8cc14f03389c2426ea3d3ddd75e12b1ec in lucene-solr's branch refs/heads/master from Uwe Schindler [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=d65c02e ] LUCENE-7595 : Disable another test not compatible with RamUsageTester
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 40a8b4edb4cfc7de5b62037fdcb389afa247573d in lucene-solr's branch refs/heads/branch_6x from Uwe Schindler
          [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=40a8b4e ]

          LUCENE-7595: Disable another test not compatible with RamUsageTester

          Show
          jira-bot ASF subversion and git services added a comment - Commit 40a8b4edb4cfc7de5b62037fdcb389afa247573d in lucene-solr's branch refs/heads/branch_6x from Uwe Schindler [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=40a8b4e ] LUCENE-7595 : Disable another test not compatible with RamUsageTester

            People

            • Assignee:
              thetaphi Uwe Schindler
              Reporter:
              thetaphi Uwe Schindler
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development