[
https://issues.apache.org/jira/browse/GEODE-10160?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Hale Bales updated GEODE-10160:
-------------------------------
Description:
Some List methods aren't overriden in SizeableByteArrayList which means that
the memoryOverhead doesn't get updated. add(int index, byte[] element),
clear(), and set(int index, byte[] newElement) all need to update the size and
don't.
This can be accomplished by adding overrides to SizeableByteArrayList:
{code:java}
@Override
public byte[] set(int index, byte[] newElement) {
byte[] replacedElement = super.set(index, newElement);
memberOverhead -= calculateByteArrayOverhead(replacedElement);
memberOverhead += calculateByteArrayOverhead(newElement);
return replacedElement;
}
@Override
public void add(int index, byte[] element) {
memberOverhead += calculateByteArrayOverhead(element);
super.add(index, element);
}
{code}
The test for set could look something like:
{code:java}
@Test
public void sizeInBytesGetsUpdatedAccurately_whenDoingSets() {
SizeableByteArrayList list = new SizeableByteArrayList();
byte[] element = "element".getBytes(StandardCharsets.UTF_8);
list.addFirst(element);
long initialSize = list.getSizeInBytes();
assertThat(initialSize).isEqualTo(sizer.sizeof(list));
list.set(0, "some really big new element
name".getBytes(StandardCharsets.UTF_8));
assertThat(list.getSizeInBytes()).isEqualTo(sizer.sizeof(list));
list.set(0, element);
assertThat(list.getSizeInBytes()).isEqualTo(initialSize);
}
{code}
We need more tests than just this one. add(int, byte[]) needs to be tested as
well. Any method on SizeableByteArrayList that modify the data should have a
test that the memoryOverhead gets updated correctly.
Clear itself isn't a problem - just set the memberOverhead to 0 - but the issue
is that for the version of LTRIM in PR#7403:
https://github.com/apache/geode/pull/7403, we clear a sublist of
SizeableByteArrayList. This means that the overridden clear method does not get
called and the LinkedList implementation of clear does not call any other
methods that we can override. There needs to be some new approach to LTRIM that
doesn't use sublists.
was:
Some List methods aren't overriden in SizeableByteArrayList which means that
the memoryOverhead doesn't get updated. add(int index, byte[] element),
clear(), and set(int index, byte[] newElement) all need to update the size and
don't.
This can be accomplished by adding overrides to SizeableByteArrayList:
{code:java}
@Override
public byte[] set(int index, byte[] newElement) {
byte[] replacedElement = super.set(index, newElement);
memberOverhead -= calculateByteArrayOverhead(replacedElement);
memberOverhead += calculateByteArrayOverhead(newElement);
return replacedElement;
}
@Override
public void add(int index, byte[] element) {
memberOverhead += calculateByteArrayOverhead(element);
super.add(index, element);
}
{code}
The test for set could look something like:
{code:java}
@Test
public void sizeInBytesGetsUpdatedAccurately_whenDoingSets() {
SizeableByteArrayList list = new SizeableByteArrayList();
byte[] element = "element".getBytes(StandardCharsets.UTF_8);
list.addFirst(element);
long initialSize = list.getSizeInBytes();
assertThat(initialSize).isEqualTo(sizer.sizeof(list));
list.set(0, "some really big new element
name".getBytes(StandardCharsets.UTF_8));
assertThat(list.getSizeInBytes()).isEqualTo(sizer.sizeof(list));
list.set(0, element);
assertThat(list.getSizeInBytes()).isEqualTo(initialSize);
}
{code}
We need more tests than just this one. add(int, byte[]) needs to be tested as
well. Any method on SizeableByteArrayList that modify the data should have a
test that the memoryOverhead gets updated correctly.
Clear itself isn't a problem - just set the memberOverhead to 0 - but the issue
is that for the version of LTRIM in
(PR#7403)[https://github.com/apache/geode/pull/7403], we clear a sublist of
SizeableByteArrayList. This means that the overridden clear method does not get
called and the LinkedList implementation of clear does not call any other
methods that we can override. There needs to be some new approach to LTRIM that
doesn't use sublists.
> SizeableByteArrayList does not update the sizeInBytes for some non-overriden
> methods
> ------------------------------------------------------------------------------------
>
> Key: GEODE-10160
> URL: https://issues.apache.org/jira/browse/GEODE-10160
> Project: Geode
> Issue Type: Bug
> Components: redis
> Affects Versions: 1.15.0
> Reporter: Hale Bales
> Priority: Major
> Labels: blocks-1.15.0
>
> Some List methods aren't overriden in SizeableByteArrayList which means that
> the memoryOverhead doesn't get updated. add(int index, byte[] element),
> clear(), and set(int index, byte[] newElement) all need to update the size
> and don't.
> This can be accomplished by adding overrides to SizeableByteArrayList:
> {code:java}
> @Override
> public byte[] set(int index, byte[] newElement) {
> byte[] replacedElement = super.set(index, newElement);
> memberOverhead -= calculateByteArrayOverhead(replacedElement);
> memberOverhead += calculateByteArrayOverhead(newElement);
> return replacedElement;
> }
> @Override
> public void add(int index, byte[] element) {
> memberOverhead += calculateByteArrayOverhead(element);
> super.add(index, element);
> }
> {code}
> The test for set could look something like:
> {code:java}
> @Test
> public void sizeInBytesGetsUpdatedAccurately_whenDoingSets() {
> SizeableByteArrayList list = new SizeableByteArrayList();
> byte[] element = "element".getBytes(StandardCharsets.UTF_8);
> list.addFirst(element);
> long initialSize = list.getSizeInBytes();
> assertThat(initialSize).isEqualTo(sizer.sizeof(list));
> list.set(0, "some really big new element
> name".getBytes(StandardCharsets.UTF_8));
> assertThat(list.getSizeInBytes()).isEqualTo(sizer.sizeof(list));
> list.set(0, element);
> assertThat(list.getSizeInBytes()).isEqualTo(initialSize);
> }
> {code}
> We need more tests than just this one. add(int, byte[]) needs to be tested as
> well. Any method on SizeableByteArrayList that modify the data should have a
> test that the memoryOverhead gets updated correctly.
> Clear itself isn't a problem - just set the memberOverhead to 0 - but the
> issue is that for the version of LTRIM in PR#7403:
> https://github.com/apache/geode/pull/7403, we clear a sublist of
> SizeableByteArrayList. This means that the overridden clear method does not
> get called and the LinkedList implementation of clear does not call any other
> methods that we can override. There needs to be some new approach to LTRIM
> that doesn't use sublists.
--
This message was sent by Atlassian Jira
(v8.20.1#820001)
