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

Faster setindex! for RHSs with inefficient linear indexing #10063

Merged
merged 1 commit into from
Feb 8, 2015

Conversation

timholy
Copy link
Member

@timholy timholy commented Feb 3, 2015

This was a long-standing TODO item, for which we now have the needed infrastructure.

Provides a 7-8x speedup on the last line of this demo:

function testset!(A, X, indexes, n)
    for i = 1:n
        setindex!(A, X, indexes...)
    end
    A
end

A = rand(1000,1001)
B = rand(800,400)
S = sub(B, 2:799, 2:399)

testset!(A, B, (101:900,101:500), 1)
testset!(A, S, (101:898,101:498), 1)

@time 1
@time testset!(A, B, (101:900,101:500), 100)
@time testset!(A, S, (101:898,101:498), 100)

@timholy
Copy link
Member Author

timholy commented Feb 7, 2015

I presume the AppVeyor failure is unrelated?

@tkelman
Copy link
Contributor

tkelman commented Feb 8, 2015

Yes, https://ci.appveyor.com/project/StefanKarpinski/julia/build/1.0.2329/job/9fx92an62in0sonp does look like an instance of #10045. Though the backtrace here (and in a few other cases, but not all) is interesting

    From worker 3:       * linalg2              in  65.09 seconds

Please submit a bug report with steps to reproduce this fault, and any error messages that follow (in their entirety). Thanks.
Exception: EXCEPTION_ACCESS_VIOLATION at 0x6e07661a -- DCABS1 at C:\projects\julia\usr\bin\libopenblas.DLL (unknown line)
DCABS1 at C:\projects\julia\usr\bin\libopenblas.DLL (unknown line)
zgemv at C:\projects\julia\usr\bin\libopenblas.DLL (unknown line)
zgemv at C:\projects\julia\usr\bin\libopenblas.DLL (unknown line)
Worker 3 terminated.    From worker 3:       * linalg3              

dcabs1 makes me think the win32 problem could possibly be the same as the 32-bit Linux problem in #10027

@timholy
Copy link
Member Author

timholy commented Feb 8, 2015

Thanks for checking, @tkelman.

timholy added a commit that referenced this pull request Feb 8, 2015
Faster setindex! for RHSs with inefficient linear indexing
@timholy timholy merged commit 2b9c024 into master Feb 8, 2015
@timholy timholy deleted the teh/fastersetindex branch February 8, 2015 11:36
@Jutho
Copy link
Contributor

Jutho commented Feb 8, 2015

I think this commit had a negative effect on the speed of setindex! as experienced in the /test/perf/cat tests. The time increase is not in cat but also in the manual performance tests, so it is in setindex!(::Array,::Array,::UnitRange{Int},::UnitRange{Int}) call.

@Jutho
Copy link
Contributor

Jutho commented Feb 8, 2015

What about writing this as:

Xs = start(X)
@nloops $N i d->(1:length(I_d)) d->(@inbounds offset_{d-1} = offset_d + (unsafe_getindex(I_d, i_d)-1)*stride_d) begin
v, Xs = next(X, Xs)
@inbounds A[offset_0] = v
end

Then fast linear indexing will be used for X if available, and cartesian indexing otherwise.

@timholy
Copy link
Member Author

timholy commented Feb 9, 2015

I'd had the impression that eachindex used a linear index when appropriate, but that is clearly wrong. (And probably reasonable, too.) So your strategy is fine. Want to do it yourself, or shall I?

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.

3 participants