beehive-lab / TornadoVM

TornadoVM: A practical and efficient heterogeneous programming framework for managed languages
https://www.tornadovm.org
Apache License 2.0
1.17k stars 110 forks source link

Refactor memory limit checks to take into account primitve type wrappers when using the `withMemoryLimit` API #352

Closed mikepapadim closed 5 months ago

mikepapadim commented 5 months ago

Description

This PR extends the support introduced in #323.

Now, memory limit checks takes into account fields along with arrays and TornadoNative types.

Backend/s tested

Mark the backends affected by this PR.

OS tested

Mark the OS where this PR is tested.

How to test the new patch?

make jdk21  BACKEND=opencl
tornado-test -V uk.ac.manchester.tornado.unittests.memoryplan.TestMemoryLimit

jjfumero commented 5 months ago

This does not correspond to the required functionality. The type of code we want to achieve is, for example, the following:

   public static void add(IntArray c) {
        for (@Parallel int i = 0; i < c.getSize(); i++) {
            c.set(i, a.get(i) + b.get(i) + value);
        }
    }

    @Test
    public void testWithMemoryLimitOver() {

        TaskGraph taskGraph = new TaskGraph("s0") //
                .task("t0", TestMemoryLimit::add, c) //
                .transferToHost(DataTransferMode.EVERY_EXECUTION, c);

        ImmutableTaskGraph immutableTaskGraph = taskGraph.snapshot();
        TornadoExecutionPlan executionPlan = new TornadoExecutionPlan(immutableTaskGraph);

        // Limit the amount of memory to be used on the target accelerator.
        executionPlan.withMemoryLimit("1GB").execute();

        for (int i = 0; i < c.getSize(); i++) {
            assertEquals(a.get(i) + b.get(i) + value, c.get(i), 0.001);
        }
        executionPlan.freeDeviceMemory();
    }

Notice that a and b are fields within the class, and the compute methods references them to access the data.

If I run this, it currently breaks:

tornado-test -V uk.ac.manchester.tornado.unittests.memoryplan.TestMemoryLimit

WARNING: Using incubator modules: jdk.incubator.vector

Test: class uk.ac.manchester.tornado.unittests.memoryplan.TestMemoryLimit
    Running test: testWithMemoryLimitOver    ................  [FAILED] 
        \_[REASON] unimplemented: address origin unimplemented: org.graalvm.compiler.nodes.ConstantNode
    Running test: testWithMemoryLimitUnder   ................  [PASS] 
    Running test: enableAndDisable           ................  [FAILED] 
        \_[REASON] Unable to allocate 314572824 bytes of memory.
Test ran: 3, Failed: 2, Unsupported: 0
mikepapadim commented 5 months ago

This does not correspond to the required functionality. The type of code we want to achieve is, for example, the following:

   public static void add(IntArray c) {
        for (@Parallel int i = 0; i < c.getSize(); i++) {
            c.set(i, a.get(i) + b.get(i) + value);
        }
    }

    @Test
    public void testWithMemoryLimitOver() {

        TaskGraph taskGraph = new TaskGraph("s0") //
                .task("t0", TestMemoryLimit::add, c) //
                .transferToHost(DataTransferMode.EVERY_EXECUTION, c);

        ImmutableTaskGraph immutableTaskGraph = taskGraph.snapshot();
        TornadoExecutionPlan executionPlan = new TornadoExecutionPlan(immutableTaskGraph);

        // Limit the amount of memory to be used on the target accelerator.
        executionPlan.withMemoryLimit("1GB").execute();

        for (int i = 0; i < c.getSize(); i++) {
            assertEquals(a.get(i) + b.get(i) + value, c.get(i), 0.001);
        }
        executionPlan.freeDeviceMemory();
    }

Notice that a and b are fields within the class, and the compute methods references them to access the data.

If I run this, it currently breaks:

tornado-test -V uk.ac.manchester.tornado.unittests.memoryplan.TestMemoryLimit

WARNING: Using incubator modules: jdk.incubator.vector

Test: class uk.ac.manchester.tornado.unittests.memoryplan.TestMemoryLimit
  Running test: testWithMemoryLimitOver    ................  [FAILED] 
      \_[REASON] unimplemented: address origin unimplemented: org.graalvm.compiler.nodes.ConstantNode
  Running test: testWithMemoryLimitUnder   ................  [PASS] 
  Running test: enableAndDisable           ................  [FAILED] 
      \_[REASON] Unable to allocate 314572824 bytes of memory.
Test ran: 3, Failed: 2, Unsupported: 0

a and b are not transferToDevice. Is this behavior supported ?

jjfumero commented 5 months ago

My bad, we need to add the transferToDevice. An exception should be thrown instead.

  @Test
    public void testWithMemoryLimitOver() {

        TaskGraph taskGraph = new TaskGraph("s0") //
                .transferToDevice(DataTransferMode.EVERY_EXECUTION, a, b) //
                .task("t0", TestMemoryLimit::add, c) //
                .transferToHost(DataTransferMode.EVERY_EXECUTION, c);

        ImmutableTaskGraph immutableTaskGraph = taskGraph.snapshot();
        TornadoExecutionPlan executionPlan = new TornadoExecutionPlan(immutableTaskGraph);

        // Limit the amount of memory to be used on the target accelerator.
        executionPlan.withMemoryLimit("1GB").execute();

        for (int i = 0; i < c.getSize(); i++) {
            assertEquals(a.get(i) + b.get(i) + value, c.get(i), 0.001);
        }
        executionPlan.freeDeviceMemory();
    }
jjfumero commented 5 months ago

As discussed, let's keep the PR as it is and open a new one with field-objects support for Panama-based arrays.