Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 2 commits into from
Mar 19, 2024

Conversation

mikepapadim
Copy link
Member

@mikepapadim mikepapadim commented Mar 13, 2024

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.

  • OpenCL
  • PTX
  • SPIRV

OS tested

Mark the OS where this PR is tested.

  • Linux
  • OSx
  • Windows

How to test the new patch?

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

@mikepapadim mikepapadim self-assigned this Mar 13, 2024
@mikepapadim mikepapadim changed the title Refactor memory limit checks to include fields in the withMemoryLimit API Refactor memory limit checks to take into account fields when using the withMemoryLimit API Mar 13, 2024
@jjfumero
Copy link
Member

jjfumero commented Mar 18, 2024

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
Copy link
Member Author

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
Copy link
Member

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
Copy link
Member

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

@mikepapadim mikepapadim changed the title Refactor memory limit checks to take into account fields when using the withMemoryLimit API Refactor memory limit checks to take into account primitve type wrappers when using the withMemoryLimit API Mar 18, 2024
@mikepapadim mikepapadim merged commit 11b8c81 into beehive-lab:develop Mar 19, 2024
1 check passed
@mikepapadim mikepapadim deleted the mikepapadim/field_checks branch March 19, 2024 09:02
jjfumero added a commit to jjfumero/TornadoVM that referenced this pull request Mar 27, 2024
Improvements
~~~~~~~~~~~~~~~~~~

- `beehive-lab#344 <https://github.com/beehive-lab/TornadoVM/pull/344>`_: Support for Multi-threaded Execution Plans.
- `beehive-lab#347 <https://github.com/beehive-lab/TornadoVM/pull/347>`_: Enhanced examples.
- `beehive-lab#350 <https://github.com/beehive-lab/TornadoVM/pull/350>`_: Obtain internal memory segment for the Tornado Native Arrays without the object header.
- `beehive-lab#357 <https://github.com/beehive-lab/TornadoVM/pull/357>`_: API extensions to query and apply filters to backends and devices from the ``TornadoExecutionPlan``.
- `beehive-lab#359 <https://github.com/beehive-lab/TornadoVM/pull/359>`_: Support Factory Methods for FFI-based array collections to be used/composed in TornadoVM Task-Graphs.

Compatibility
~~~~~~~~~~~~~~~~~~

- `beehive-lab#351 <https://github.com/beehive-lab/TornadoVM/pull/351>`_: Compatibility of TornadoVM Native Arrays with the Java Vector API.
- `beehive-lab#352 <https://github.com/beehive-lab/TornadoVM/pull/352>`_: Refactor memory limit to take into account primitive types and wrappers.
- `beehive-lab#354 <https://github.com/beehive-lab/TornadoVM/pull/354>`_: Add DFT-sample benchmark in FP32.
- `beehive-lab#356 <https://github.com/beehive-lab/TornadoVM/pull/356>`_: Initial support for Windows 11 using Visual Studio Development tools.
- `beehive-lab#361 <https://github.com/beehive-lab/TornadoVM/pull/361>`_: Compatibility with the SPIR-V toolkit v0.0.4.
- `beehive-lab#366 <https://github.com/beehive-lab/TornadoVM/pull/363>`_: Level Zero JNI Dependency updated to 0.1.3.

Bug Fixes
~~~~~~~~~~~~~~~~~~

- `beehive-lab#346 <https://github.com/beehive-lab/TornadoVM/pull/346>`_: Computation of local-work group sizes for the Level Zero/SPIR-V backend fixed.
- `beehive-lab#360 <https://github.com/beehive-lab/TornadoVM/pull/358>`_: Fix native tests to check the JIT compiler for each backend.
- `beehive-lab#355 <https://github.com/beehive-lab/TornadoVM/pull/355>`_: Fix custom exceptions when a driver/device is not found.
@jjfumero jjfumero mentioned this pull request Mar 27, 2024
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants