Nov11 / kryo

Automatically exported from code.google.com/p/kryo
BSD 3-Clause "New" or "Revised" License
0 stars 0 forks source link

IntMap.clear() does not work as expected #144

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago
IntMap.clear() does not make a map empty. I run into this bug while 
experimenting with the DefaultClassResolver class who uses this method.

Here is a unit test to reproduce a problem:

package com.esotericsoftware.kryo;

import java.io.ByteArrayInputStream;
import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.sql.Date;
import java.util.ArrayList;
import java.util.IdentityHashMap;
import java.util.List;

import com.esotericsoftware.kryo.Kryo;
import com.esotericsoftware.kryo.io.Input;
import com.esotericsoftware.kryo.io.Output;
import com.esotericsoftware.kryo.util.IntMap;
import com.esotericsoftware.minlog.Log;

public class IntMapTest extends KryoTestCase {

    public void testCase () throws Exception {
        IntMap<Object> map = new IntMap<Object>();
        map.put(9, new Object());
        map.clear();
        assertNull("Key 9 should not be present", map.get(9));
        System.out.println(map);
    }
}

My current proposal for a fix would be this:
    public void clear () {
        int[] keyTable = this.keyTable;
        V[] valueTable = this.valueTable;

        for (int i = capacity + stashSize; i-- > 0;) {
            keyTable[i] = EMPTY;
            valueTable[i] = null;
        }

//      for (int i = size - 1; i >= 0; i--) {
//          keyTable[i] = EMPTY;
//          valueTable[i] = null;
//      }
//      for (int i = capacity, n = i + stashSize - 1; i < n; i++) {
//          keyTable[i] = EMPTY;
//          valueTable[i] = null;
//      }
        size = 0;
        stashSize = 0;
        zeroValue = null;
        hasZeroValue = false;
    }

But I'm wondering what is the current logic in the code which I commented out? 
May be there is a more efficient way to clear a map than what I suggested above?

@Nate: Do you remember why it is written this way currently?

Original issue reported on code.google.com by romixlev on 22 Oct 2013 at 7:53

GoogleCodeExporter commented 8 years ago
Aye, it is currently wrong. Should be like this:
https://github.com/libgdx/libgdx/blob/master/gdx/src/com/badlogic/gdx/utils/IntM
ap.java#L414

Original comment by nathan.s...@gmail.com on 22 Oct 2013 at 8:04

GoogleCodeExporter commented 8 years ago

Original comment by romixlev on 22 Oct 2013 at 8:10