dominion-dev / dominion-ecs-java

Insanely fast ECS (Entity Component System) for Java
https://dominion-dev.github.io
MIT License
288 stars 17 forks source link

When I create entities and add components in multiple threads, there are unforeseen concurrency issues #165

Closed endison1986 closed 1 year ago

endison1986 commented 1 year ago

In my actual project, I create monster entities in a System. In order to take advantage of multi-core, I use parallelStream and create monster entities in it, and cache the Entity to Component, but I found that in the next System, the Entity in the Component obtained by the context.findEntitiesWith() method is different from the hashCode of ResultSet.entity()

example code like this.

public enum State {
    S1, S2, S3;
}

public static class C {
    private final Entity entity;

    public C(Entity entity) {
        this.entity = entity;
    }
}

public static class B {

}

public static class A {
    private C c;

    public A(Dominion dominion) {
        c = new C(dominion.createEntity(this));
        c.entity.setState(S1);
    }
}

public static void main(String[] args) {
    final Dominion dominion = Dominion.create();
    final var list = List.<Runnable>of(() -> {
        for (int i = 0; i < 200; i++) {
            final var a = new A(dominion);
            a.c.entity.add(new B());
        }
    }, () -> {
        for (int i = 0; i < 200; i++) {
            final var a = new A(dominion);
            a.c.entity.add(new B());
        }
    }, () -> {
        for (int i = 0; i < 200; i++) {
            final var a = new A(dominion);
            a.c.entity.add(new B());
        }
    });
    final var scheduler = dominion.createScheduler();
    scheduler.schedule(() -> list.parallelStream().forEach(Runnable::run));
    scheduler.schedule(() -> {
        dominion.findEntitiesWith(A.class).stream().forEach(rs->{
            if(rs.entity().hashCode() != rs.comp().c.entity.hashCode()) {
                System.out.println("1111111111111111111");
                System.exit(-1);
            }
        });
        System.exit(0);
    });
    scheduler.tickAtFixedRate(10);
}

If I remove a.c.entity.add(new B());, the program is fine. If I change paralleStream() to stream(), this program is fine.

endison1986 commented 1 year ago

when I call rs.entity().getId() in System, I found that the ID of Entity has many repetitions, and the chunkId in the ID still points to the old chunk.

scheduler.schedule(() -> {
    dominion.findEntitiesWith(A.class).stream().forEach(rs->{
        System.out.println(((IntEntity)rs.entity()).getId());
    });
    System.exit(0);
});

this is log

8791,8790,8789,8788,8787,8786,8785,8784,8783,8782,8781,8780,8779,8778,8777,8776,8775,8774,8773,8772,8771,8770,8769,8768,8767,8766,8765,8764,8763,8762,8761,8760,8759,8758,8757,8756,8755,8754,8753,8752,8751,4098,8749,8748,8747,8746,8745,8744,8743,8742,8741,8740,8739,4097,8737,8736,8735,8734,8733,8732,8731,8730,8729,8728,8727,8726,8725,8724,4098,8722,8721,8720,8719,8718,4097,8716,8715,8714,8713,8712,8711,8710,8709,4097,8707,4098,8705,8704,4097,8702,8701,8700,8699,8698,8697,8696,8695,8694,8693,8692,4097,8690,8689,8688,8687,8686,8685,4098,8683,8682,8681,8680,8679,8678,8677,8676,8675,8674,8673,8672,4098,8670,4097,8668,4097,8666,8665,8664,8663,8662,8661,8660,8659,4098,8657,4098,4097,8654,8653,8652,8651,4098,8649,8648,4097,8646,8645,8644,8643,8642,8641,8640,8639,8638,4097,8636,8635,8634,8633,8632,8631,4097,8629,8628,8627,8626,8625,8624,8623,4097,8621,8620,8619,8618,8617,4098,8615,8614,8613,4097,8611,8610,8609,8608,8607,8606,8605,8604,8603,8602,8601,8600,8599,8598,8597,8596,8595,8594,8593,8592,8591,8590,4098,4097,8587,8586,8585,8584,8583,8582,8581,8580,8579,8578,8577,4098,4097,8574,8573,4097,8571,8570,8569,8568,8567,8566,8565,8564,8563,8562,8561,8560,8559,4097,8557,8556,8555,8554,8553,8552,8551,8550,8549,8548,4097,8546,8545,8544,8543,8542,8541,8540,8539,8538,8537,8536,4097,8534,8533,8532,8531,8530,8529,8528,8527,8526,8525,8524,8523,8522,8521,8520,8519,8518,4098,8516,8515,8514,8513,8512,8511,8510,8509,8508,8507,8506,8505,8504,8503,8502,4098,8500,4098,8498,8497,8496,8495,8494,4097,8492,8491,8490,8489,4098,8487,8486,4097,8484,4098,8482,8481,8480,8479,8478,8477,8476,4098,4097,8473,8472,8471,8470,8469,8468,4097,8466,8465,8464,8463,8462,8461,4097,8459,8458,8457,8456,8455,8454,8453,8452,8451,8450,4098,8448,8447,8446,8445,8444,8443,8442,8441,8440,8439,8438,8437,8436,8435,8434,8433,4098,8431,8430,8429,8428,8427,8426,8425,8424,8423,8422,8421,8420,8419,8418,8417,4098,8415,8414,8413,8412,8411,8410,8409,8408,8407,8406,8405,8404,8403,8402,8401,8400,8399,8398,8397,8396,8395,8394,8393,8392,8391,4098,8389,8388,8387,8386,8385,8384,8383,4098,8381,8380,4098,8378,8377,4098,8375,8374,8373,8372,8371,8370,8369,8368,8367,8366,8365,8364,8363,8362,8361,8360,8359,8358,8357,8356,8355,8354,8353,8352,8351,8350,4098,8348,8347,8346,8345,8344,8343,8342,8341,8340,8339,8338,8337,8336,8335,8334,8333,8332,8331,8330,8329,8328,8327,8326,8325,8324,8323,8322,4098,4097,8319,8318,8317,8316,4098,8314,8313,4097,8311,8310,8309,4097,8307,8306,8305,8304,8303,8302,8301,4097,8299,8298,4098,8296,8295,8294,8293,8292,8291,4098,8289,8288,4098,8286,8285,8284,8283,4098,8281,8280,8279,8278,8277,8276,4097,8274,8273,8272,8271,8270,4098,8268,8267,8266,8265,8264,4098,8262,8261,4097,8259,8258,4098,8256,8255,8254,8253,8252,8251,8250,8249,8248,8247,8246,8245,8244,8243,4098,8241,4097,8239,8238,8237,8236,8235,8234,8233,8232,8231,4097,8229,4097,8227,8226,8225,8224,4098,8222,8221,4098,8219,8218,4097,8216,4098,8214,8213,8212,8211,8210,8209,8208,8207,8206,8205,8204,8203,8202,8201,4098,8199,8198,8197,8196,8195,8194,4098,8192

you will find many 4097 and 4098

endison1986 commented 1 year ago

hi, @enricostara I found that this is a very troublesome concurrency issue. this issue may appear in the two methods of remove and copy. since remove will move the Item, the copied data may be wrong. I am not sure if this is the case, and I have not yet known how to fix this issue.

enricostara commented 1 year ago

Hi @endison1986 , I'm still investigating this issue

enricostara commented 1 year ago

Hi @endison1986 , please checkout and double-check if it works as expected. In IntEntityTest I added a unit test derived from your example code

endison1986 commented 1 year ago

Hi, @enricostara First of all I must thank you for your work. Second, I run my project and my previous test code, they all work fine. But I modified my test code and found some problems, I added unit test in IntEntity

323 @Test
324    public void concurrentConcatenatedAdd2() throws InterruptedException {
325//        System.setProperty("dominion.logging-level", "TRACE");
326//        System.setProperty("dominion.test.logging-level", "TRACE");
327
328        ExecutorService executorService = Executors.newFixedThreadPool(10);
329        EntityRepository entityRepository = (EntityRepository) new EntityRepository.Factory().create("stress-test");
330//        EntityRepository entityRepository = (EntityRepository) new EntityRepository.Factory().create("test");
331
332        AtomicInteger counter = new AtomicInteger(1);
333        Runnable runnable = () -> {
334            for (int i = 0; i < 10000; i++) {
335                int id = counter.getAndIncrement();
336                entityRepository.createEntity(new C1(id)).add(new C2(id));
337            }
338        };
339        Runnable runnable1 = () -> {
340            for (int i = 0; i < 10000; i++) {
341                int id = counter.getAndIncrement();
342                entityRepository.createEntity(new C1(id)).add(new C2(id)).add(new C3(id));
343            }
344        };
345        executorService.execute(runnable);
346        executorService.execute(runnable1);
347
348        executorService.shutdown();
349        Assertions.assertTrue(executorService.awaitTermination(5, TimeUnit.SECONDS));
350
351        entityRepository.findEntitiesWith(C1.class).stream().forEach(rs -> {
352            Assertions.assertNotNull(rs.entity());
353            Assertions.assertNotNull(rs.comp());
354        });
355    }

I get an error message

org.opentest4j.AssertionFailedError: expected: not <null>
<6 internal lines>
at dev.dominion.ecs.test.engine/dev.dominion.ecs.test.engine.IntEntityTest.lambda$concurrentConcatenatedAdd2$13(IntEntityTest.java:352)
at java.base/java.util.Iterator.forEachRemaining(Iterator.java:133)<2 internal lines>
at dev.dominion.ecs.test.engine/dev.dominion.ecs.test.engine.IntEntityTest.concurrentConcatenatedAdd2(IntEntityTest.java:351)<31 internal lines>
at java.base/java.util.ArrayList.forEach(ArrayList.java:1511) <9 internal lines>
at java.base/java.util.ArrayList.forEach(ArrayList.java:1511) <27 internal lines>
enricostara commented 1 year ago

Hi @endison1986 , I added the new test and improved the code. Please check again if It works as expected.

endison1986 commented 1 year ago

Hi @enricostara , it all works fine in my project and test code, thanks.

enricostara commented 1 year ago

Thanks you for your tests!