You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@geode.apache.org by "mcmellawatt (GitHub)" <gi...@apache.org> on 2018/09/12 17:44:35 UTC
[GitHub] [geode] mcmellawatt opened pull request #2459: GEODE-5725:
Using string values to exercise off-heap
Thank you for submitting a contribution to Apache Geode.
In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:
### For all changes:
- [X] Is there a JIRA ticket associated with this PR? Is it referenced in the commit message?
- [X] Has your PR been rebased against the latest commit within the target branch (typically `develop`)?
- [X] Is your initial contribution a single, squashed commit?
- [X] Does `gradlew build` run cleanly?
- [X] Have you written or updated unit tests to verify your changes?
- [X] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)?
### Note:
Please ensure that once the PR is submitted, you check travis-ci for build issues and
submit an update to your PR as soon as possible. If you need help, please send an
email to dev@geode.apache.org.
[ Full content available at: https://github.com/apache/geode/pull/2459 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode] mcmellawatt closed pull request #2459: GEODE-5725:
Using string values to exercise off-heap
Posted by "mcmellawatt (GitHub)" <gi...@apache.org>.
[ pull request closed by mcmellawatt ]
[ Full content available at: https://github.com/apache/geode/pull/2459 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode] mcmellawatt commented on issue #2459: GEODE-5725:
Using string values to exercise off-heap
Posted by "mcmellawatt (GitHub)" <gi...@apache.org>.
> A more descriptive message as to why this change is desired would be great. As it is, the change looks OK but I wouldn't know why.
>
> Looking at the JIRA, it seems we need the value to be > 8 bytes. What is the actual size in bytes of these string values?
@pdxrunner - would "Using <8 bit values to exercise off-heap in tests" be better? The "why" is basically because we were not exercising off-heap when we thought we were in our WAN DUnit tests as described in the Jira, but I prefer to keep commit message short and succinct. If they want the details, they can read the Jira ticket.
Regarding the size of these string values, they are serialized as modified UTF-8 with two bytes of length information. The minimum length of the string is 8 chars: (7 for "Value_") + (1 for smallest long.toString() e.g. '0') + null terminator = 8 chars. In its serialized form, this results in 10 byte values at a minimum, which is larger than the 8 byte cutoff for off-heap to be exercised. Of course if once the long hits more than 1 character the size will increase, but the value will be 10 bytes minimum which is sufficient for these tests.
[ Full content available at: https://github.com/apache/geode/pull/2459 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode] pdxrunner commented on issue #2459: GEODE-5725:
Using string values to exercise off-heap
Posted by "pdxrunner (GitHub)" <gi...@apache.org>.
> > A more descriptive message as to why this change is desired would be great. As it is, the change looks OK but I wouldn't know why.
> > Looking at the JIRA, it seems we need the value to be > 8 bytes. What is the actual size in bytes of these string values?
>
> @pdxrunner - would "Using <8 bit values to exercise off-heap in tests" be better? The "why" is basically because we were not exercising off-heap when we thought we were in our WAN DUnit tests as described in the Jira, but I prefer to keep commit message short and succinct. If they want the details, they can read the Jira ticket.
>
> Regarding the size of these string values, they are serialized as modified UTF-8 with two bytes of length information. The minimum length of the string is 8 chars: (7 for "Value_") + (1 for smallest long.toString() e.g. '0') + null terminator = 8 chars. In its serialized form, this results in 10 byte values at a minimum, which is larger than the 8 byte cutoff for off-heap to be exercised. Of course if once the long hits more than 1 character the size will increase, but the value will be 10 bytes minimum which is sufficient for these tests.
@mcmellawatt I was thinking of the discussion thread on the dev-list regarding descriptive commit messages. The gist as I see it is that when reviewing the history of code changes (i.e. `git blame` or equivalent) that reviewing the commit logs should provide enough without having to flip back and forth between browsing JIRA and the IDE or git log.
For this case I think something like "Use values large enough (> 8 bytes) to force off-heap storage" would fit the bill for this simple change.
[ Full content available at: https://github.com/apache/geode/pull/2459 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode] mcmellawatt commented on issue #2459: GEODE-5725:
Using string values to exercise off-heap
Posted by "mcmellawatt (GitHub)" <gi...@apache.org>.
> A more descriptive message as to why this change is desired would be great. As it is, the change looks OK but I wouldn't know why.
>
> Looking at the JIRA, it seems we need the value to be > 8 bytes. What is the actual size in bytes of these string values?
@pdxrunner - would "Using <8 bit values to exercise off-heap in tests" be better? The why is basically because we were not exercising off-heap when we thought we were in our WAN DUnit tests as described in the Jira, but I prefer to keep commit message short and succinct. If they want the details, they can read the Jira ticket.
Regarding the size of these string values, they are serialized as modified UTF-8 with two bytes of length information. The minimum length of the string is 8 chars: (7 for "Value_") + (1 for smallest long.toString() e.g. '0') + null terminator = 8 chars. In its serialized form, this results in 10 byte values at a minimum, which is larger than the 8 byte cutoff for off-heap to be exercised. Of course if once the long hits more than 1 character the size will increase, but the value will be 10 bytes minimum which is sufficient for these tests.
[ Full content available at: https://github.com/apache/geode/pull/2459 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode] mcmellawatt commented on issue #2459: GEODE-5725:
Using string values to exercise off-heap
Posted by "mcmellawatt (GitHub)" <gi...@apache.org>.
Will do, thanks Ken!
[ Full content available at: https://github.com/apache/geode/pull/2459 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode] pdxrunner commented on issue #2459: GEODE-5725:
Using string values to exercise off-heap
Posted by "pdxrunner (GitHub)" <gi...@apache.org>.
A more descriptive message as to why this change is desired would be great. As it is, the change looks OK but I wouldn't know why.
Looking at the JIRA, it seems we need the value to be > 8 bytes. What is the actual size in bytes of these string values?
[ Full content available at: https://github.com/apache/geode/pull/2459 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org