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

Add slice access #53

Merged
merged 1 commit into from
Feb 16, 2020
Merged

Add slice access #53

merged 1 commit into from
Feb 16, 2020

Conversation

willglynn
Copy link
Contributor

@willglynn willglynn commented Sep 30, 2019

I added slice access to specs (amethyst/specs#634) to support OpenCL (amethyst/specs#553). Usage looks like:

impl<'a> System<'a> for SomeSystem {
    type SystemData = (WriteStorage<'a, Something>, );

    fn run(&mut self, (mut some_storage, ): Self::SystemData) {
        // 1. get a mutable slice of the component storage
        let slice = some_storage.unprotected_storage_mut().as_mut_slice();

        // 2. process the slice with OpenCL or whatever

        // 3. there is no step 3
    }
}

Components can be processed using an OpenCL kernel:

struct Something {
    float foo;
    float bar;
    float baz;
};

__kernel void process_something(__global struct Something* slice) {
    __global struct Something* something = &slice[get_global_id(0)];
    // do stuff with this data
}

There's a catch when using slices from DefaultVecStorage<T> or VecStorage<T>. The slices are sparse, which means the kernel will either uselessly process T::default() or perform unchecked access into a MaybeUninit<T> for any entity which does not have component T.

Exposing a slice from hibitset::BitSet would allow an OpenCL kernel check the storage mask itself:

struct Something {
    float foo;
    float bar;
    float baz;
};

#define BITS_PER_SIZE_T (sizeof(size_t) * 8)

__kernel void process_something(__global size_t* bitset_layer0, __global struct Something* slice) {
    // check that this index contains data
    size_t mask_index = get_global_id(0) / BITS_PER_SIZE_T;
    size_t shift = get_global_id(0) % BITS_PER_SIZE_T;
    if ((bitset_layer0[mask_index] >> shift) & 1 == 0) {
        return;
    }
  
    __global struct Something* something = &slice[get_global_id(0)];
    // do stuff with this data
}

This PR adds layer{0,1,2}_as_slice(&self) -> &[usize] to BitSet, as well as trivial constants which make correct usage more obvious. Layer 3 is specified to be a single usize so I didn't see any benefit to exposing it as a slice.

Copy link
Member

@azriel91 azriel91 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Heya, thank you for this, the docs are really clear ✌️

@azriel91
Copy link
Member

bors r+

bors bot added a commit that referenced this pull request Feb 16, 2020
53: Add slice access r=azriel91 a=willglynn

I added slice access to `specs` (amethyst/specs#634) to support OpenCL (amethyst/specs#553). Usage looks like:

```rust
impl<'a> System<'a> for SomeSystem {
    type SystemData = (WriteStorage<'a, Something>, );

    fn run(&mut self, (mut some_storage, ): Self::SystemData) {
        // 1. get a mutable slice of the component storage
        let slice = some_storage.unprotected_storage_mut().as_mut_slice();

        // 2. process the slice with OpenCL or whatever

        // 3. there is no step 3
    }
}
```

Components can be processed using an OpenCL kernel:

```c
struct Something {
    float foo;
    float bar;
    float baz;
};

__kernel void process_something(__global struct Something* slice) {
    __global struct Something* something = &slice[get_global_id(0)];
    // do stuff with this data
}
```

There's a catch when using slices from `DefaultVecStorage<T>` or `VecStorage<T>`. The slices are sparse, which means the kernel will either uselessly process `T::default()` or perform unchecked access into a `MaybeUninit<T>` for any entity which does not have component `T`.

Exposing a slice from `hibitset::BitSet` would allow an OpenCL kernel check the storage mask itself:

```c
struct Something {
    float foo;
    float bar;
    float baz;
};

#define BITS_PER_SIZE_T (sizeof(size_t) * 8)

__kernel void process_something(__global size_t* bitset_layer0, __global struct Something* slice) {
    // check that this index contains data
    size_t mask_index = get_global_id(0) / BITS_PER_SIZE_T;
    size_t shift = get_global_id(0) % BITS_PER_SIZE_T;
    if ((bitset_layer0[mask_index] >> shift) & 1 == 0) {
        return;
    }
  
    __global struct Something* something = &slice[get_global_id(0)];
    // do stuff with this data
}
```

This PR adds `layer{0,1,2}_as_slice(&self) -> &[usize]` to `BitSet`, as well as trivial constants which make correct usage more obvious. Layer 3 is specified to be a single `usize` so I didn't see any benefit to exposing it as a slice.

Co-authored-by: Will Glynn <[email protected]>
@bors
Copy link
Contributor

bors bot commented Feb 16, 2020

Build succeeded

@bors bors bot merged commit ff41a6d into amethyst:master Feb 16, 2020
@willglynn willglynn deleted the slice_access branch February 17, 2020 02:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants