Skip to content

Commit

Permalink
codegen: fix type bug in dereferenceable_size (#24553)
Browse files Browse the repository at this point in the history
fix #24511
  • Loading branch information
vtjnash authored and JeffBezanson committed Nov 9, 2017
1 parent 27665e8 commit 26eb512
Showing 1 changed file with 4 additions and 2 deletions.
6 changes: 4 additions & 2 deletions src/cgutils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -335,9 +335,11 @@ static size_t dereferenceable_size(jl_value_t *jt)
if (jl_is_array_type(jt)) {
// Array has at least this much data
return sizeof(jl_array_t);
} else if (((jl_datatype_t*)jt)->layout) {
}
else if (jl_is_datatype(jt) && ((jl_datatype_t*)jt)->layout) {
return jl_datatype_size(jt);
} else {
}
else {
return 0;
}
}
Expand Down

14 comments on commit 26eb512

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

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

Executing the daily benchmark build, I will reply here when finished:

@nanosoldier runbenchmarks(ALL, isdaily = true)

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

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

Your benchmark job has completed - successfully executed benchmarks. A full report can be found here. cc @ararslan

@ararslan
Copy link
Member

Choose a reason for hiding this comment

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

HE'S BAAAAAAAAACK

@KristofferC
Copy link
Member

Choose a reason for hiding this comment

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

What's the reason it didn't do a comparison against the previous recorded benchmark?

@ararslan
Copy link
Member

Choose a reason for hiding this comment

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

There are no previous JSON benchmarks. Before that, the daily data was in JLD, and after the switch there was an issue where the daily builds weren't running. That's been resolved, and this is the first daily since the resolution, so tomorrow's will compare against this.

@KristofferC
Copy link
Member

Choose a reason for hiding this comment

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

Make sense. Great to have it back!

@fredrikekre
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps we should try to run it against latest daily somehow, to see what happened during all the weeks it was down?

@KristofferC
Copy link
Member

Choose a reason for hiding this comment

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

@nanosoldier runbenchmarks(ALL, vs = "@fce2d689222c10ae735362dc67a8f6b973be55a3")

@KristofferC
Copy link
Member

Choose a reason for hiding this comment

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

Let's see if this works...

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

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

Your benchmark job has completed - possible performance regressions were detected. A full report can be found here. cc @ararslan

@KristofferC
Copy link
Member

Choose a reason for hiding this comment

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

Parsing JSON should probably be looked at.

@andreasnoack
Copy link
Member

Choose a reason for hiding this comment

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

It also looks like something has happened to sparse-dense multiplication

@vtjnash
Copy link
Member Author

Choose a reason for hiding this comment

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

That benchmark (sparse) appears to just be noisy.

The notable regressions I see appear to be:
["linalg", "arithmetic", ("*", "Tridiagonal", "Vector", 1024)]
Julia Parse
JSON Parse

@Sacha0
Copy link
Member

@Sacha0 Sacha0 commented on 26eb512 Nov 10, 2017

Choose a reason for hiding this comment

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

Yes, ignore the sparse-dense matmul benchmarks :).

Please sign in to comment.