[jira] Created: (DERBY-3731) Improve calculation of refSize in ClassSize.java

Improve calculation of refSize in ClassSize.java
-------------------------------------------------

                 Key: DERBY-3731
                 URL: https://issues.apache.org/jira/browse/DERBY-3731
             Project: Derby
          Issue Type: Bug
          Components: SQL
            Reporter: Kathey Marsden
            Priority: Minor


java/engine/org/apache/derby/iapi/services/cache/ClassSize.java has a static code block which calculates the size of a reference for the architecture.  This code could be improved by adding garbage collection before measuring memory, to give a consistent  reading.     Also there have been suggestions that we use os.arch or sun.arch.data.model to make the measurement more reliable, especially on 64bit machines.


--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

     [ https://issues.apache.org/jira/browse/DERBY-3731?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Kathey Marsden updated DERBY-3731:
----------------------------------

    Attachment: DERBY-3731_diff.txt

Attached is an initial patch (DERBY-3731_diff.txt)  for this issue that adds the garbage collection before reading totalMemory()  and freeMemory(). This I would like to check into trunk and backport to 10.4 and 10.3.   Any additional changes for using os.arch can go in subsequent to this and probably should only go into the trunk as they might destabilize query plan selection. Running tests now.

> Improve calculation of refSize in ClassSize.java
> -------------------------------------------------
>
>                 Key: DERBY-3731
>                 URL: https://issues.apache.org/jira/browse/DERBY-3731
>             Project: Derby
>          Issue Type: Bug
>          Components: SQL
>            Reporter: Kathey Marsden
>            Priority: Minor
>         Attachments: DERBY-3731_diff.txt
>
>
> java/engine/org/apache/derby/iapi/services/cache/ClassSize.java has a static code block which calculates the size of a reference for the architecture.  This code could be improved by adding garbage collection before measuring memory, to give a consistent  reading.     Also there have been suggestions that we use os.arch or sun.arch.data.model to make the measurement more reliable, especially on 64bit machines.

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

    [ https://issues.apache.org/jira/browse/DERBY-3731?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12606927#action_12606927 ]

Knut Anders Hatlen commented on DERBY-3731:
-------------------------------------------

Not directly relevant for this issue, but I'm posting some
observations here so that they are not lost.

I used DTrace to check how much memory that was allocated in JDK 6 on
OpenSolaris 2008.05 and compared the results to what ClassSize would
estimate. I found these differences (assuming that refSize was
correctly set to 4 on 32-bit VMs and 8 on 64-bit VMs):

1) a better name for minObjectSize would be minArraySize, as it
attempts to estimate the fixed overhead for arrays

2) minObjectSize is set to 4*refSize (16 bytes on 32-bit, 32 bytes on
64-bit). The actual value is 3*refSize, but because of 8-byte word
boundaries the size of an empty array is refSize*3+4 == 4*refSize on
32-bit VMs (and the unused space can be exploited, so byte[0] and
byte[4] use the exact same number of bytes). On 64-bit VMs, 3*refSize
matches the word boundaries, so the correct value is 24 (and byte[1]
uses 8 more bytes than byte[0]).

3) booleanSize should be 1, not 4

4) charSize and shortSize should be 2, not 4

5) estimateBaseFromCoefficients() uses minObjectSize as the minimum
object size, whereas it actually is objectOverhead*refSize.

> Improve calculation of refSize in ClassSize.java
> -------------------------------------------------
>
>                 Key: DERBY-3731
>                 URL: https://issues.apache.org/jira/browse/DERBY-3731
>             Project: Derby
>          Issue Type: Bug
>          Components: SQL
>            Reporter: Kathey Marsden
>            Priority: Minor
>         Attachments: DERBY-3731_diff.txt
>
>
> java/engine/org/apache/derby/iapi/services/cache/ClassSize.java has a static code block which calculates the size of a reference for the architecture.  This code could be improved by adding garbage collection before measuring memory, to give a consistent  reading.     Also there have been suggestions that we use os.arch or sun.arch.data.model to make the measurement more reliable, especially on 64bit machines.

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

    [ https://issues.apache.org/jira/browse/DERBY-3731?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12607246#action_12607246 ]

Knut Anders Hatlen commented on DERBY-3731:
-------------------------------------------

Although sun.arch.data.model is not a required property, it is present in both Sun's and IBM's JVMs.

> Improve calculation of refSize in ClassSize.java
> -------------------------------------------------
>
>                 Key: DERBY-3731
>                 URL: https://issues.apache.org/jira/browse/DERBY-3731
>             Project: Derby
>          Issue Type: Bug
>          Components: SQL
>            Reporter: Kathey Marsden
>            Priority: Minor
>         Attachments: DERBY-3731_diff.txt
>
>
> java/engine/org/apache/derby/iapi/services/cache/ClassSize.java has a static code block which calculates the size of a reference for the architecture.  This code could be improved by adding garbage collection before measuring memory, to give a consistent  reading.     Also there have been suggestions that we use os.arch or sun.arch.data.model to make the measurement more reliable, especially on 64bit machines.

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

    [ https://issues.apache.org/jira/browse/DERBY-3731?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12611342#action_12611342 ]

Kathey Marsden commented on DERBY-3731:
---------------------------------------

ported fix to do gc before memory estimation to 10.4 and 10.3. Leaving the issue open for possible other improvements.

> Improve calculation of refSize in ClassSize.java
> -------------------------------------------------
>
>                 Key: DERBY-3731
>                 URL: https://issues.apache.org/jira/browse/DERBY-3731
>             Project: Derby
>          Issue Type: Bug
>          Components: SQL
>            Reporter: Kathey Marsden
>            Priority: Minor
>         Attachments: DERBY-3731_diff.txt
>
>
> java/engine/org/apache/derby/iapi/services/cache/ClassSize.java has a static code block which calculates the size of a reference for the architecture.  This code could be improved by adding garbage collection before measuring memory, to give a consistent  reading.     Also there have been suggestions that we use os.arch or sun.arch.data.model to make the measurement more reliable, especially on 64bit machines.

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

    [ https://issues.apache.org/jira/browse/DERBY-3731?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12613257#action_12613257 ]

Kristian Waagan commented on DERBY-3731:
----------------------------------------

Relying on Runtime.gc is not the best solution in my opinion and the call can even be disabled on the command line (at least for the Sun JVMs), causing it to have no effect. I've seen this done in appserver environments to reduce impact of ill-behaved applications doing explicit GC.
The calculations are also sensitive to other kinds of activities in the JVM, so I'd like to see further improvements to  these calculations.

I don't know much about this code, but I see Knut Anders has already posted a list of potential issues.
It seems feasible to code an algorithm using the following mechanisms to determine the refSize (i.e. 32 or 64 bit system):
 1) Use system property "sun.arch.data.model"
 2) Use system property "os.arch" (requires some more logic/investigation)
 3) Fall back to the existing logic.

If nobody else feels the itch, I can make a change proposal for the refSize calculation, and if accepted, see how that affects our tests.

> Improve calculation of refSize in ClassSize.java
> -------------------------------------------------
>
>                 Key: DERBY-3731
>                 URL: https://issues.apache.org/jira/browse/DERBY-3731
>             Project: Derby
>          Issue Type: Bug
>          Components: SQL
>            Reporter: Kathey Marsden
>            Priority: Minor
>         Attachments: DERBY-3731_diff.txt
>
>
> java/engine/org/apache/derby/iapi/services/cache/ClassSize.java has a static code block which calculates the size of a reference for the architecture.  This code could be improved by adding garbage collection before measuring memory, to give a consistent  reading.     Also there have been suggestions that we use os.arch or sun.arch.data.model to make the measurement more reliable, especially on 64bit machines.

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

     [ https://issues.apache.org/jira/browse/DERBY-3731?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Kristian Waagan updated DERBY-3731:
-----------------------------------

    Attachment: derby-3731-1a-refsize_from_properties.diff

'derby-3731-1a-refsize_from_properties.diff' is a first shot at
determining the reference size by looking at system properties.

I got the following results from a run on my own machine when specifying
-d64 for the VM, using Sun JDK 1.6:
### model: 8 (sun.arch.data.model)
### arch amd64: 8 (os.arch)
### sz: -26 (sz calculated by heuristic)
### REFERENCE SIZE: 4 (final value calculated by heuristic)

As you can see, in this case the estimation was off and the results are
reproducible.  However, when I tried the same with JDK 1.5, the
estimation was correct.

There's a few things I haven't checked with the patch:
 a) Do we need some extra permissions?
    I was able to run with the default policy file.
 b) Do we need to document the permissions?
 c) Should the code be more defensive in making sure the data model code
    returns either 4 or 8?
 d) Is using 'os.arch' worth the hassle?
 e) How many values are there for 'os.arch'?

Comments on this issue (and the patch) are welcome.
If people think it is a safe enough change, I would suggest to commit
the final version and see how it works out with the test runs. We have
individuals running off trunk, we have the tinderbox and the nighly
tests.
I also believe we have people developing on various platforms
(Solaris, Linux, Windows, Mac, maybe more).

> Improve calculation of refSize in ClassSize.java
> -------------------------------------------------
>
>                 Key: DERBY-3731
>                 URL: https://issues.apache.org/jira/browse/DERBY-3731
>             Project: Derby
>          Issue Type: Bug
>          Components: SQL
>            Reporter: Kathey Marsden
>            Priority: Minor
>         Attachments: derby-3731-1a-refsize_from_properties.diff, DERBY-3731_diff.txt
>
>
> java/engine/org/apache/derby/iapi/services/cache/ClassSize.java has a static code block which calculates the size of a reference for the architecture.  This code could be improved by adding garbage collection before measuring memory, to give a consistent  reading.     Also there have been suggestions that we use os.arch or sun.arch.data.model to make the measurement more reliable, especially on 64bit machines.

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

     [ https://issues.apache.org/jira/browse/DERBY-3731?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Kristian Waagan updated DERBY-3731:
-----------------------------------

    Derby Info: [Patch Available]

> Improve calculation of refSize in ClassSize.java
> -------------------------------------------------
>
>                 Key: DERBY-3731
>                 URL: https://issues.apache.org/jira/browse/DERBY-3731
>             Project: Derby
>          Issue Type: Bug
>          Components: SQL
>            Reporter: Kathey Marsden
>            Priority: Minor
>         Attachments: derby-3731-1a-refsize_from_properties.diff, DERBY-3731_diff.txt
>
>
> java/engine/org/apache/derby/iapi/services/cache/ClassSize.java has a static code block which calculates the size of a reference for the architecture.  This code could be improved by adding garbage collection before measuring memory, to give a consistent  reading.     Also there have been suggestions that we use os.arch or sun.arch.data.model to make the measurement more reliable, especially on 64bit machines.

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

    [ https://issues.apache.org/jira/browse/DERBY-3731?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12615971#action_12615971 ]

Knut Anders Hatlen commented on DERBY-3731:
-------------------------------------------

I think the patch looks good. We can always add more os.arch values later as we discover new ones. Since os.arch is defined by the spec, I think it is a good thing to check it if sun.arch.data.model is not found or not accessible. Later, if this change doesn't cause any problems, should we also mention these properties in the documentation and put them in the default policy installed by the network server?

One small suggestion to make the code slightly simpler: The two loops in the checking of os.arch could be replaced by
  if (Arrays.asList(b32).contains(arch)) return 4;
and
  if (Arrays.asList(b64).contains(arch)) return 8;

And maybe the code to get the value of a system property could be moved into a separate method to save some code duplication.

Should the THROWASSERT() be replaced with a DEBUG_PRINT() so that we don't prevent people from running the debug builds on platforms not used by the developers?

> Improve calculation of refSize in ClassSize.java
> -------------------------------------------------
>
>                 Key: DERBY-3731
>                 URL: https://issues.apache.org/jira/browse/DERBY-3731
>             Project: Derby
>          Issue Type: Bug
>          Components: SQL
>            Reporter: Kathey Marsden
>            Priority: Minor
>         Attachments: derby-3731-1a-refsize_from_properties.diff, DERBY-3731_diff.txt
>
>
> java/engine/org/apache/derby/iapi/services/cache/ClassSize.java has a static code block which calculates the size of a reference for the architecture.  This code could be improved by adding garbage collection before measuring memory, to give a consistent  reading.     Also there have been suggestions that we use os.arch or sun.arch.data.model to make the measurement more reliable, especially on 64bit machines.

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

    [ https://issues.apache.org/jira/browse/DERBY-3731?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12615981#action_12615981 ]

Knut Anders Hatlen commented on DERBY-3731:
-------------------------------------------

Logged DERBY-3794 for the problems with some other estimates in ClassSize.

> Improve calculation of refSize in ClassSize.java
> -------------------------------------------------
>
>                 Key: DERBY-3731
>                 URL: https://issues.apache.org/jira/browse/DERBY-3731
>             Project: Derby
>          Issue Type: Bug
>          Components: SQL
>            Reporter: Kathey Marsden
>            Priority: Minor
>         Attachments: derby-3731-1a-refsize_from_properties.diff, DERBY-3731_diff.txt
>
>
> java/engine/org/apache/derby/iapi/services/cache/ClassSize.java has a static code block which calculates the size of a reference for the architecture.  This code could be improved by adding garbage collection before measuring memory, to give a consistent  reading.     Also there have been suggestions that we use os.arch or sun.arch.data.model to make the measurement more reliable, especially on 64bit machines.

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

     [ https://issues.apache.org/jira/browse/DERBY-3731?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Kristian Waagan reassigned DERBY-3731:
--------------------------------------

    Assignee: Kristian Waagan

> Improve calculation of refSize in ClassSize.java
> -------------------------------------------------
>
>                 Key: DERBY-3731
>                 URL: https://issues.apache.org/jira/browse/DERBY-3731
>             Project: Derby
>          Issue Type: Bug
>          Components: SQL
>            Reporter: Kathey Marsden
>            Assignee: Kristian Waagan
>            Priority: Minor
>         Attachments: derby-3731-1a-refsize_from_properties.diff, DERBY-3731_diff.txt
>
>
> java/engine/org/apache/derby/iapi/services/cache/ClassSize.java has a static code block which calculates the size of a reference for the architecture.  This code could be improved by adding garbage collection before measuring memory, to give a consistent  reading.     Also there have been suggestions that we use os.arch or sun.arch.data.model to make the measurement more reliable, especially on 64bit machines.

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

    [ https://issues.apache.org/jira/browse/DERBY-3731?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12615999#action_12615999 ]

Kristian Waagan commented on DERBY-3731:
----------------------------------------

Thanks for looking at the patch, Knut Anders.
Your comments are all valid, I'll incorporate them in the next revision.

I investigated a little more regarding the permissions. Seems Derby isn't allowed to read sun.arch.data.model with the current policy file, but it is allowed to read os.arch for some reason.
Adding code to print the current permissions shows that reading the property os.arch is indeed granted, but it is not in our default policy file. Since I don't know if this is specific for the Sun JVM or not, maybe we should document and add both permissions to the default policy file?

> Improve calculation of refSize in ClassSize.java
> -------------------------------------------------
>
>                 Key: DERBY-3731
>                 URL: https://issues.apache.org/jira/browse/DERBY-3731
>             Project: Derby
>          Issue Type: Bug
>          Components: SQL
>            Reporter: Kathey Marsden
>            Priority: Minor
>         Attachments: derby-3731-1a-refsize_from_properties.diff, DERBY-3731_diff.txt
>
>
> java/engine/org/apache/derby/iapi/services/cache/ClassSize.java has a static code block which calculates the size of a reference for the architecture.  This code could be improved by adding garbage collection before measuring memory, to give a consistent  reading.     Also there have been suggestions that we use os.arch or sun.arch.data.model to make the measurement more reliable, especially on 64bit machines.

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

     [ https://issues.apache.org/jira/browse/DERBY-3731?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Kristian Waagan updated DERBY-3731:
-----------------------------------

    Attachment: derby-3731-1b-refsize_from_properties.stat
                derby-3731-1b-refsize_from_properties.diff

Patch 1b addresses the suggestions from Knut Anders and it adds permissions for the two system properties to the default server policy file and the testing policy file.

Documentation must be added later.
Patch ready for another review.

> Improve calculation of refSize in ClassSize.java
> -------------------------------------------------
>
>                 Key: DERBY-3731
>                 URL: https://issues.apache.org/jira/browse/DERBY-3731
>             Project: Derby
>          Issue Type: Bug
>          Components: SQL
>            Reporter: Kathey Marsden
>            Assignee: Kristian Waagan
>            Priority: Minor
>         Attachments: derby-3731-1a-refsize_from_properties.diff, derby-3731-1b-refsize_from_properties.diff, derby-3731-1b-refsize_from_properties.stat, DERBY-3731_diff.txt
>
>
> java/engine/org/apache/derby/iapi/services/cache/ClassSize.java has a static code block which calculates the size of a reference for the architecture.  This code could be improved by adding garbage collection before measuring memory, to give a consistent  reading.     Also there have been suggestions that we use os.arch or sun.arch.data.model to make the measurement more reliable, especially on 64bit machines.

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

    [ https://issues.apache.org/jira/browse/DERBY-3731?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12616428#action_12616428 ]

Knut Anders Hatlen commented on DERBY-3731:
-------------------------------------------

I downloaded the 1b patch and tried to run some of the tests that have had unstable query plans without noticing any problems (tried both on a 32-bit JVM and on a 64-bit JVM). The changes look good to me, +1 to commit.

By the way, should we catch NumberFormatException when parsing the data model property so that we don't prevent users from booting Derby if there is a platform that sets this property to something that's not a number?

> Improve calculation of refSize in ClassSize.java
> -------------------------------------------------
>
>                 Key: DERBY-3731
>                 URL: https://issues.apache.org/jira/browse/DERBY-3731
>             Project: Derby
>          Issue Type: Bug
>          Components: SQL
>            Reporter: Kathey Marsden
>            Assignee: Kristian Waagan
>            Priority: Minor
>         Attachments: derby-3731-1a-refsize_from_properties.diff, derby-3731-1b-refsize_from_properties.diff, derby-3731-1b-refsize_from_properties.stat, DERBY-3731_diff.txt
>
>
> java/engine/org/apache/derby/iapi/services/cache/ClassSize.java has a static code block which calculates the size of a reference for the architecture.  This code could be improved by adding garbage collection before measuring memory, to give a consistent  reading.     Also there have been suggestions that we use os.arch or sun.arch.data.model to make the measurement more reliable, especially on 64bit machines.

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

    [ https://issues.apache.org/jira/browse/DERBY-3731?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12616439#action_12616439 ]

Kristian Waagan commented on DERBY-3731:
----------------------------------------

Thanks for the testing, Knut Anders.

Yes, catching the NFE should be done. I have found out that the property can be set to "unknown". I'm also wondering if we should enforce 4 and 8 as return values to avoid any surprises on "exotic VMs".

> Improve calculation of refSize in ClassSize.java
> -------------------------------------------------
>
>                 Key: DERBY-3731
>                 URL: https://issues.apache.org/jira/browse/DERBY-3731
>             Project: Derby
>          Issue Type: Bug
>          Components: SQL
>            Reporter: Kathey Marsden
>            Assignee: Kristian Waagan
>            Priority: Minor
>         Attachments: derby-3731-1a-refsize_from_properties.diff, derby-3731-1b-refsize_from_properties.diff, derby-3731-1b-refsize_from_properties.stat, DERBY-3731_diff.txt
>
>
> java/engine/org/apache/derby/iapi/services/cache/ClassSize.java has a static code block which calculates the size of a reference for the architecture.  This code could be improved by adding garbage collection before measuring memory, to give a consistent  reading.     Also there have been suggestions that we use os.arch or sun.arch.data.model to make the measurement more reliable, especially on 64bit machines.

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

    [ https://issues.apache.org/jira/browse/DERBY-3731?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12616443#action_12616443 ]

Knut Anders Hatlen commented on DERBY-3731:
-------------------------------------------

The old code doesn't enforce 4 or 8, but it does make sure that refSize
is at least 4. Perhaps the new code should do the same? I don't think we
need to do anything more advanced than that. If the JVM tells us it's
using a 97-bit data model, why should we assume it's lying? ;)

> Improve calculation of refSize in ClassSize.java
> -------------------------------------------------
>
>                 Key: DERBY-3731
>                 URL: https://issues.apache.org/jira/browse/DERBY-3731
>             Project: Derby
>          Issue Type: Bug
>          Components: SQL
>            Reporter: Kathey Marsden
>            Assignee: Kristian Waagan
>            Priority: Minor
>         Attachments: derby-3731-1a-refsize_from_properties.diff, derby-3731-1b-refsize_from_properties.diff, derby-3731-1b-refsize_from_properties.stat, DERBY-3731_diff.txt
>
>
> java/engine/org/apache/derby/iapi/services/cache/ClassSize.java has a static code block which calculates the size of a reference for the architecture.  This code could be improved by adding garbage collection before measuring memory, to give a consistent  reading.     Also there have been suggestions that we use os.arch or sun.arch.data.model to make the measurement more reliable, especially on 64bit machines.

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.