From 057fa2aa9f9a3d6e41e14ab08e70fa995eff3122 Mon Sep 17 00:00:00 2001 From: Jeff Bezanson Date: Thu, 27 Apr 2017 19:09:25 -0400 Subject: [PATCH] small optimization to `f(...; kw...)` when `kw` is empty --- base/essentials.jl | 20 +++++++ base/task.jl | 2 +- base/test.jl | 3 + src/julia-syntax.scm | 135 ++++++++++++++++++++++++------------------- test/test.jl | 2 + 5 files changed, 100 insertions(+), 62 deletions(-) diff --git a/base/essentials.jl b/base/essentials.jl index 145ea6bb50486..c5322273ba4a5 100644 --- a/base/essentials.jl +++ b/base/essentials.jl @@ -336,6 +336,26 @@ function vector_any(xs::ANY...) a end +function as_kwargs(xs::Union{AbstractArray,Associative}) + n = length(xs) + to = Vector{Any}(n*2) + i = 1 + for (k, v) in xs + to[i] = k::Symbol + to[i+1] = v + i += 2 + end + return to +end + +function as_kwargs(xs) + to = Vector{Any}(0) + for (k, v) in xs + ccall(:jl_array_ptr_1d_push2, Void, (Any, Any, Any), to, k::Symbol, v) + end + return to +end + isempty(itr) = done(itr, start(itr)) """ diff --git a/base/task.jl b/base/task.jl index c2172c4ce7d53..5639bb444a739 100644 --- a/base/task.jl +++ b/base/task.jl @@ -209,7 +209,7 @@ function task_done_hook(t::Task) if isa(t.donenotify, Condition) && !isempty(t.donenotify.waitq) handled = true - notify(t.donenotify, result, error=err) + notify(t.donenotify, result, true, err) end # Execute any other hooks registered in the TLS diff --git a/base/test.jl b/base/test.jl index e7c6e5afe75ce..a64c9679b0264 100644 --- a/base/test.jl +++ b/base/test.jl @@ -848,6 +848,9 @@ function testset_beginend(args, tests) # action (such as reporting the results) quote ts = $(testsettype)($desc; $options...) + # this empty loop is here to force the block to be compiled, + # which is needed for backtrace scrubbing to work correctly. + while false; end push_testset(ts) try $(esc(tests)) diff --git a/src/julia-syntax.scm b/src/julia-syntax.scm index 50a0a67e9a1de..842327fe740d7 100644 --- a/src/julia-syntax.scm +++ b/src/julia-syntax.scm @@ -493,7 +493,7 @@ (= ,ii (call (top -) (call (top *) ,i 2) 1)) (= ,elt (call (core arrayref) ,kw ,ii)) ,(foldl (lambda (kvf else) - (let* ((k (car kvf)) + (let* ((k (car kvf)) (rval0 `(call (core arrayref) ,kw (call (top +) ,ii 1))) ;; note: if the "declared" type of a KW arg @@ -536,9 +536,9 @@ (list `(... ,(arg-name (car vararg)))))) ;; otherwise add to rest keywords `(foreigncall 'jl_array_ptr_1d_push (core Void) (call (core svec) Any Any) - ,rkw 0 (tuple ,elt - (call (core arrayref) ,kw - (call (top +) ,ii 1))) 0)) + ,rkw 0 (tuple ,elt + (call (core arrayref) ,kw + (call (top +) ,ii 1))) 0)) (map list vars vals flags)))) ;; set keywords that weren't present to their default values ,@(apply append @@ -1437,64 +1437,77 @@ (reverse a)))))) ;; lower function call containing keyword arguments -(define (lower-kw-call fexpr kw pa) - (let ((container (make-ssavalue))) - (let loop ((kw kw) - (initial-kw '()) ;; keyword args before any splats - (stmts '()) - (has-kw #f)) ;; whether there are definitely >0 kwargs - (if (null? kw) - (let ((f (if (sym-ref? fexpr) fexpr (make-ssavalue)))) +(define (lower-kw-call fexpr kw0 pa) + + (define (kwcall-unless-empty f pa kw-container-test kw-container) + (let* ((expr_stmts (remove-argument-side-effects `(call ,f ,@pa))) + (pa (cddr (car expr_stmts))) + (stmts (cdr expr_stmts))) + `(block + ,@stmts + (if (call (top isempty) ,kw-container-test) + (call ,f ,@pa) + (call (call (core kwfunc) ,f) ,kw-container ,f ,@pa))))) + + (let ((f (if (sym-ref? fexpr) fexpr (make-ssavalue)))) + `(block + ,@(if (eq? f fexpr) '() `((= ,f, fexpr))) + ,(if ;; optimize splatting one existing container, `f(...; kw...)` + (and (length= kw0 1) (vararg? (car kw0))) + (let* ((container (cadr (car kw0))) + (expr_stmts (remove-argument-side-effects `(call _ ,container))) + (container (caddr (car expr_stmts))) + (stmts (cdr expr_stmts))) `(block - ,@(if (eq? f fexpr) '() `((= ,f, fexpr))) - ,(if (null? stmts) - `(call (call (core kwfunc) ,f) (call (top vector_any) ,@(reverse initial-kw)) ,f ,@pa) - `(block - (= ,container (call (top vector_any) ,@(reverse initial-kw))) - ,@(reverse stmts) - ,(if has-kw - `(call (call (core kwfunc) ,f) ,container ,f ,@pa) - (let* ((expr_stmts (remove-argument-side-effects `(call ,f ,@pa))) - (pa (cddr (car expr_stmts))) - (stmts (cdr expr_stmts))) - `(block - ,@stmts - (if (call (top isempty) ,container) - (call ,f ,@pa) - (call (call (core kwfunc) ,f) ,container ,f ,@pa))))))))) - (let ((arg (car kw))) - (cond ((and (pair? arg) (eq? (car arg) 'parameters)) - (error "more than one semicolon in argument list")) - ((kwarg? arg) - (if (not (symbol? (cadr arg))) - (error (string "keyword argument is not a symbol: \"" - (deparse (cadr arg)) "\""))) - (if (vararg? (caddr arg)) - (error "splicing with \"...\" cannot be used for a keyword argument value")) - (if (null? stmts) - (loop (cdr kw) (list* (caddr arg) `(quote ,(cadr arg)) initial-kw) stmts #t) - (loop (cdr kw) initial-kw - (cons `(foreigncall 'jl_array_ptr_1d_push2 (core Void) (call (core svec) Any Any Any) - ,container 0 - (|::| (quote ,(cadr arg)) (core Symbol)) 0 - ,(caddr arg) 0) - stmts) - #t))) - (else - (loop (cdr kw) initial-kw - (cons (let* ((k (make-ssavalue)) - (v (make-ssavalue)) - (push-expr `(foreigncall 'jl_array_ptr_1d_push2 (core Void) (call (core svec) Any Any Any) - ,container 0 - (|::| ,k (core Symbol)) 0 - ,v 0))) - (if (vararg? arg) - `(for (= (tuple ,k ,v) ,(cadr arg)) - ,push-expr) - `(block (= (tuple ,k ,v) ,arg) - ,push-expr))) - stmts) - (or has-kw (not (vararg? arg))))))))))) + ,@stmts + ,(kwcall-unless-empty f pa container `(call (top as_kwargs) ,container)))) + (let ((container (make-ssavalue))) + (let loop ((kw kw0) + (initial-kw '()) ;; keyword args before any splats + (stmts '()) + (has-kw #f)) ;; whether there are definitely >0 kwargs + (if (null? kw) + (if (null? stmts) + `(call (call (core kwfunc) ,f) (call (top vector_any) ,@(reverse initial-kw)) ,f ,@pa) + `(block + (= ,container (call (top vector_any) ,@(reverse initial-kw))) + ,@(reverse stmts) + ,(if has-kw + `(call (call (core kwfunc) ,f) ,container ,f ,@pa) + (kwcall-unless-empty f pa container container)))) + (let ((arg (car kw))) + (cond ((and (pair? arg) (eq? (car arg) 'parameters)) + (error "more than one semicolon in argument list")) + ((kwarg? arg) + (if (not (symbol? (cadr arg))) + (error (string "keyword argument is not a symbol: \"" + (deparse (cadr arg)) "\""))) + (if (vararg? (caddr arg)) + (error "splicing with \"...\" cannot be used for a keyword argument value")) + (if (null? stmts) + (loop (cdr kw) (list* (caddr arg) `(quote ,(cadr arg)) initial-kw) stmts #t) + (loop (cdr kw) initial-kw + (cons `(foreigncall 'jl_array_ptr_1d_push2 (core Void) (call (core svec) Any Any Any) + ,container 0 + (|::| (quote ,(cadr arg)) (core Symbol)) 0 + ,(caddr arg) 0) + stmts) + #t))) + (else + (loop (cdr kw) initial-kw + (cons (let* ((k (make-ssavalue)) + (v (make-ssavalue)) + (push-expr `(foreigncall 'jl_array_ptr_1d_push2 (core Void) (call (core svec) Any Any Any) + ,container 0 + (|::| ,k (core Symbol)) 0 + ,v 0))) + (if (vararg? arg) + `(for (= (tuple ,k ,v) ,(cadr arg)) + ,push-expr) + `(block (= (tuple ,k ,v) ,arg) + ,push-expr))) + stmts) + (or has-kw (not (vararg? arg)))))))))))))) ;; convert e.g. A'*B to Ac_mul_B(A,B) (define (expand-transposed-op e ops) diff --git a/test/test.jl b/test/test.jl index 3fbc5f9a96ed3..01898c234e93c 100644 --- a/test/test.jl +++ b/test/test.jl @@ -416,6 +416,8 @@ end io = IOBuffer() @test (print(io, Base.Test.Error(:test_error, "woot", 5, backtrace())); 1) == 1 str = String(take!(io)) +# NOTE: This test depends on the code generated by @testset getting compiled, +# to get good backtraces. If it fails, check the implementation of `testset_beginend`. @test contains(str, "test.jl") @test !contains(str, "boot.jl")