Skip to content

Commit

Permalink
fix a compiler front-end performance bug
Browse files Browse the repository at this point in the history 
This was causing the lowering pass to be run twice on method definitions,
which takes, well, twice as long. This is really embarrassing, but
I'm going to commit it anyway.
  • Loading branch information
@JeffBezanson
JeffBezanson committed Aug 13, 2014
1 parent e625e75 commit 454344f
Showing 1 changed file with 2 additions and 1 deletion.
3 changes: 2 additions & 1 deletion src/toplevel.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -450,7 +450,8 @@ jl_value_t *jl_toplevel_eval_flex(jl_value_t *e, int fast)
int ewc = 0;
JL_GC_PUSH3(&thunk, &thk, &ex);

if (ex->head != body_sym && ex->head != thunk_sym) {
if (ex->head != body_sym && ex->head != thunk_sym && ex->head != return_sym &&
ex->head != method_sym) {
// not yet expanded
ex = (jl_expr_t*)jl_expand(e);
}
Expand Down

8 comments on commit 454344f

@kmsquire
Copy link
Member

Choose a reason for hiding this comment

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

This definitely deserves to be backported to v0.3! Thanks!

@Keno
Copy link
Member

@Keno Keno commented on 454344f Aug 13, 2014

Choose a reason for hiding this comment

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

Already has been.

@timholy
Copy link
Member

Choose a reason for hiding this comment

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

Nice!

@sjkelly
Copy link
Contributor

Choose a reason for hiding this comment

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

Jeff, our lab owes you a dinner! I'd say the amount of people that have been inspired by Julia significantly outweighs the amount that might have been bothered by this bug. I was reading the issue last night, and the quality of teamwork was outstanding. This is an amazing community and I appreciate all your guys work.

@JeffBezanson
Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks! Working with everybody here is truly enjoyable. Without @timholy 's particular test case, I never noticed this problem. Loading his generated file of 1000 functions showed everything called 2000 times, which was the key to figuring this out.

@timholy
Copy link
Member

Choose a reason for hiding this comment

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

Glad to be of use, and of course it all would have been for naught without Jeff diving in.

@jey
Copy link
Contributor

@jey jey commented on 454344f Aug 13, 2014

Choose a reason for hiding this comment

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

I'm curious, why does the parser perform "early" expansion at all?

@jey
Copy link
Contributor

@jey jey commented on 454344f Aug 13, 2014

Choose a reason for hiding this comment

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

Oh, probably because it saves on flisp/julia round trips

Please sign in to comment.