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

RFC: Improved implementation of SubArray #3503

Closed
wants to merge 2 commits into from
Closed

RFC: Improved implementation of SubArray #3503

wants to merge 2 commits into from

Conversation

lindahua
Copy link
Contributor

Based on the discussion in #3496

@lindahua
Copy link
Contributor Author

It is just in the status of initial setup. I appreciate your feedback as this work proceeds.

@lindahua
Copy link
Contributor Author

I agree with allowing dim > ndims(a).

I am going to first work on the main functionality. It is easy to add bound checking afterwards, when the main interface stabilize.

@lindahua
Copy link
Contributor Author

lindahua commented Jul 5, 2013

There is a potential safety issue here:

Consider the code below:

a = rand(100)

s = sub(a, 1:10)
push!(a, 2.0)   

x = s[1]   # this can be dangerous, as the base address of a may have been changed during push!

Even when though s maintains a reference to a which prevents a from being released, s.ptr can become invalid after an element is pushed to a (this operation may or may not change the base address of the pointer).

@lindahua
Copy link
Contributor Author

lindahua commented Jul 5, 2013

I suspect that we don't have to cache the pointer for performance when Jeff's optimization work is ready.

@ViralBShah
Copy link
Member

@JeffBezanson Do you think it is ok to go ahead with this approach for now, to improve SubArray indexing?

@lindahua Why not use ArrayView instead of MultiDimView as a name?

This will also give a fast implementation for #3701 and help move towards the new design.

@timholy
Copy link
Sponsor Member

timholy commented Jul 21, 2013

@ViralBShah, I think we still need some additional technology. See #1917 (comment)

@ViralBShah
Copy link
Member

@StefanKarpinski What do we do here, given the new work on array views?

@ViralBShah
Copy link
Member

I suggest we should close this and focus our attention on views.

@ViralBShah
Copy link
Member

While this PR implements views, what I mean is to focus on getting our indexing operations to return views rather than copies, and work on making that really fast.

@StefanKarpinski
Copy link
Sponsor Member

I've been working on some fast linear indexing for multidimensional array views. Should have something to share soon.

@lindahua
Copy link
Contributor Author

Close. My latest efforts are at #5513.

@lindahua lindahua closed this Jan 24, 2014
@tkelman tkelman deleted the dl/subview branch October 26, 2015 16:59
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.

None yet

5 participants