decouple inference from Base #11274

vtjnash · 2015-05-14T20:10:04Z

having inference inside Base made it hard to modify and slow to compile the sysimg since bootstrapping made two complete passes over the code. additionally, it made inference vulnerable to sabotage -- intentional or unintentional -- by the users program.

to combat those issues, this creates a copy of the essential code required for running the inference into Core.Inference. this allows any part of the system (esp. Base) around inference to change, and even be recompiled or monkey-patched, without impacting the core inference algorithms.

@carnaval this also implies that you can build sysimg.jl without Core.Inference present, and that you can change the inference module in use at runtime.

inspired by @ihnorton's subversive activities in #11148

vtjnash · 2015-05-14T21:10:02Z

test/core.jl

@@ -1164,7 +1164,7 @@ foo4129(a::Baz4129,b::Bar41291,args...) = foo4129(a,b.f,b,args...)
 foo4129(a::Baz4129,b::Bar41292,args...) = foo4129(a,b.f,b,args...)
 foo4129(a::Baz4129,args...)         = foo4129(a,a.b,args...)

-@test isa(foo4129(Baz4129(Bar41291(Foo4129())),1,2), Tuple{Baz4129,Bar4129,Foo4129,Int,Int})
+#@test isa(foo4129(Baz4129(Bar41291(Foo4129())),1,2), Tuple{Baz4129,Bar4129,Foo4129,Int,Int})


@carnaval any idea why this change would have broken this test and reintroduced #4129?

nevermind, i made a silly typo

timholy · 2015-05-14T21:20:01Z

💯 Will make it much easier to make disruptive changes, where getting inference working can be the main bottleneck.

I bet this speeds make clean && make quite a bit, as well as changes to files that aren't included in coreimg.jl.

tkelman · 2015-05-14T23:14:31Z

strange that julia-debug is segfaulting on the travis branch build but not the pr merge build?

tkelman · 2015-05-15T02:41:24Z

Some seemingly unrelated changes to inlining costs here?

Might be nice to not let contrib/build_{sysimg,executable}.jl bitrot too badly.

vtjnash · 2015-05-15T03:13:13Z

Some seemingly unrelated changes to inlining costs here?

i removed support for floating point numbers and rewrote the algorithm in terms of integers. it should give the same answers however.

Might be nice to not let contrib/build_{sysimg,executable}.jl bitrot too badly.

good point. in general, we might want to actually distribute the inference.ji file. although bootstrapping off an existing sys.ji file may also work.

tkelman · 2015-05-15T03:26:12Z

i removed support for floating point numbers and rewrote the algorithm in terms of integers

Oh. Right. Not so unrelated then.

tkelman · 2015-05-15T05:25:19Z

Looks like this cuts off a good 2.5 minutes off of each AppVeyor build. Not too shabby.

carnaval · 2015-05-15T14:59:28Z

Had a quick look. The inference changes look reasonable (i.e. no semantic changes). I'm mostly afraid of all the potential places where it could be assumed that whatever the top module is, it is at least always the same. The possibilities of more flexible inference and faster bootstrap are very enticing however.

StefanKarpinski · 2015-05-15T16:54:30Z

Is there any way to know about the top module assumptions other than merging and seeing what happens? Btw, I've been in favor of changing the top module from Main to a separate Root module for some time now (which is ironic since that's how Jeff originally had it and I convinced him to just use Main as the root).

Keno · 2015-05-15T16:57:36Z

src/codegen.cpp

@@ -1282,27 +1282,7 @@ jl_value_t *jl_static_eval(jl_value_t *ex, void *ctx_, jl_module_t *mod,
            jl_value_t *f = jl_static_eval(jl_exprarg(e,0),ctx,mod,sp,ast,sparams,allow_alloc);
            if (f && jl_is_function(f)) {
                jl_fptr_t fptr = ((jl_function_t*)f)->fptr;
-                if (fptr == &jl_apply_generic) {


What's up with this case getting removed?

this was support for a pre 0.1 deprecation from when ccall could only handle constant pointers. i removed jl_base_module, so i figured it was sensible to remove this special case also.

Ok, can this go on master to cut down the diff?

Sure. Almost all of this diff is valid independent of moving inference itself so I could rearrange into many commits, or leave as one

do you have a preference, or can i just merge this?

I'd rather this be split out as a separate commit to keep commits atomic.

how's this now, better?

i think so anyways.

Yes, thanks.

I quite like the granularity now, and the really descriptive commit messages are awesome too.

vtjnash · 2015-05-15T19:48:07Z

I'm mostly afraid of all the potential places where it could be assumed that whatever the top module is, it is at least always the same

Is there any way to know about the top module assumptions other than merging and seeing what happens?

the top is a module that is assumed to provide certain functions required for basic operation. there is no documented list of these, but there are a few in inference.jl itself (to provide basic constant folding for some important operations) and a few in julia-parser (such as unsafe_cconvert). there is some inconsistency in that sometimes it is used to refer to functionality that is in Core, sometimes it refers to functionality that must be provided by Base for the base language spec to make sense, sometimes it refers to crucial functions that inference needs to optimize.

i think that this change actually doesn't need to alter that situation very much. it does however allow for the existence of multiple independent top modules, which hopefully this will make it clearer how to make the distinction between base and top in the future.

vtjnash · 2015-05-17T03:57:56Z

travis broken because of 7ec501d6#commitcomment-11228450

tkelman · 2015-05-19T07:27:47Z

Is Warning: replacing module Inference expected?

vtjnash · 2015-05-19T10:39:50Z

Yes. And it ooks like this (and #11343) are ready to merge.

ihnorton · 2015-05-19T11:56:00Z

+:100:

tkelman · 2015-05-19T14:14:59Z

Odd to have a warning during normal expected operation. That'll confuse people.

this was a deprecation to support efficient code generation for ccall(dlsym(x,dlopen(y)),...) from pre-0.1 this syntax is not encouraged and has not been in use for several years

making inference separate from base required modifying the definition of TopNode to be more general, since Base may not even exist at points and it is desirable for Core.Inference to be wholly separate from Base. rather than special-casing Base, this introduces the concept that any module can indicate that it is valid as a top module and will define appropriate functions, as expected by the julia parser, to make this legal. At the start of the module definition, modules can ccall jl_set_istopmod(isprimary) to indicate this. The primary top module will be used to resolve the top module for any code that is not contained in another top module. Only one module can be the primary top module at a time, typically it will be Base.

…ed for floating point numbers in inference.

… and t_ffunc::Array (for builtin functions) this removes the dependence on being able to serialize ObjectIdDict from the inference code

decouple inference from Base

ihnorton · 2015-05-20T14:25:46Z

YOLO

sjkelly · 2015-05-20T14:26:44Z

YOMO! (You Only Merge Once)

ihnorton · 2015-05-20T14:29:48Z

Cross-referencing #5155 as this makes the "modularize base in-place" discussion vastly more approachable.

(@sjkelly indeed 😄)

tkelman · 2015-05-20T17:19:59Z

We should really get rid of that warning.

ihnorton · 2015-05-21T02:57:03Z

We should really get rid of that warning.

@tkelman done (1b6f9be)

timholy · 2015-05-29T15:28:01Z

I notice that when one is messing with files that are necessary for inference (e.g., tuple.jl), inference is always compiled twice. I'm not sure that was true formerly---if you didn't have to resort to make clean I think make would just compile things once. Is this more correct, or something that could be further optimized (safely)?

tkelman · 2015-05-29T15:34:06Z

Maybe if inference0.ji already exists, then that step could be made to skip the first stage build?

vtjnash · 2015-05-29T21:15:25Z

It's more correct, although rarely necessary (unless it impacts the results of inference). But I didn't altogether mind discouraging altering of those files, and even eventually shrinking them further :)

These were broken by #11274 but no one noticed. Fixes #13350.

These were broken by #11274 but no one noticed. Fixes #13350. (cherry picked from commit 7dfcf70) ref #13355

These were broken by #11274 but no one noticed. Fixes #13350.

These were broken by JuliaLang#11274 but no one noticed. Fixes JuliaLang#13350.

vtjnash force-pushed the jn/coreimg branch from 221d642 to bf51b16 Compare May 14, 2015 20:40

vtjnash reviewed May 14, 2015
View reviewed changes

vtjnash force-pushed the jn/coreimg branch from bf51b16 to b75ce1d Compare May 15, 2015 00:59

vtjnash changed the title ~~WIP: decouple inference from Base~~ decouple inference from Base May 15, 2015

ScottPJones mentioned this pull request May 15, 2015

Fix Unicode bugs with UTF-16/UTF-32 conversions (#10959) #11004

Closed

Keno reviewed May 15, 2015
View reviewed changes

vtjnash force-pushed the jn/coreimg branch 2 times, most recently from bd96e25 to 3218a0f Compare May 16, 2015 05:05

vtjnash mentioned this pull request May 19, 2015

fix object_id usage in dump deserializer #11337

Merged

vtjnash force-pushed the jn/coreimg branch from 3218a0f to 164010f Compare May 19, 2015 05:09

tkelman mentioned this pull request May 19, 2015

Remove .bat files from contrib/windows #11343

Merged

vtjnash added 5 commits May 19, 2015 23:32

remove jl_static_eval special-case for dlsym/dlopen

18fa555

this was a deprecation to support efficient code generation for ccall(dlsym(x,dlopen(y)),...) from pre-0.1 this syntax is not encouraged and has not been in use for several years

rearrange base to make several files a bit more stand-alone

ae6acf4

switch inlining costs to use scaled integer math. this removes the ne…

677f537

…ed for floating point numbers in inference.

split t_func::ObjectIdDict to t_ifunc::Array (for IntrinsicFunctions)…

0e88529

… and t_ffunc::Array (for builtin functions) this removes the dependence on being able to serialize ObjectIdDict from the inference code

ihnorton added a commit that referenced this pull request May 20, 2015

Merge pull request #11274 from JuliaLang/jn/coreimg

4c2f908

decouple inference from Base

ihnorton merged commit 4c2f908 into master May 20, 2015

This was referenced May 20, 2015

Speed up compilation by 20 seconds #11361

Closed

Add keyword argument and functors support to invoke (invoke improvement No. 4) #11165

Closed

Throw exceptions with arguments without creating a GC frame #11284

Closed

yuyichao mentioned this pull request May 22, 2015

SegFault in PyCall #11395

Closed

timholy mentioned this pull request May 22, 2015

WIP: static compile part 4 (user-interface) #8745

Merged

tkelman deleted the jn/coreimg branch May 22, 2015 23:58

hayd mentioned this pull request May 23, 2015

File name conventions for modules in Base? #11302

Closed

rened mentioned this pull request May 31, 2015

build_sysimg() broken #11513

Closed

simonster mentioned this pull request Jun 30, 2015

Fix a hang in type inference after workspace() #11948

Merged

vtjnash mentioned this pull request Jul 30, 2015

userimg: Reuse existing sys.[ji,so]? #9470

Closed

simonster added a commit that referenced this pull request Sep 29, 2015

Fix inference heuristic hacks

7dfcf70

These were broken by #11274 but no one noticed. Fixes #13350.

simonster mentioned this pull request Sep 29, 2015

Fix inference heuristic hacks #13355

Merged

simonster added a commit that referenced this pull request Sep 30, 2015

Fix inference heuristic hacks

fef1fb8

These were broken by #11274 but no one noticed. Fixes #13350. (cherry picked from commit 7dfcf70) ref #13355

simonster added a commit that referenced this pull request Oct 22, 2015

Fix inference heuristic hacks

d1e1fc3

These were broken by #11274 but no one noticed. Fixes #13350.

simonster added a commit that referenced this pull request Oct 22, 2015

Fix inference heuristic hacks

9822e93

These were broken by #11274 but no one noticed. Fixes #13350.

simonster added a commit that referenced this pull request Oct 22, 2015

Fix inference heuristic hacks

f8d2294

These were broken by #11274 but no one noticed. Fixes #13350.

bjarthur pushed a commit to bjarthur/julia that referenced this pull request Oct 27, 2015

Fix inference heuristic hacks

98f74d6

These were broken by JuliaLang#11274 but no one noticed. Fixes JuliaLang#13350.

simonster mentioned this pull request Dec 16, 2015

Inlining arithmetic can produce slowdowns #13350

Closed

timholy mentioned this pull request Jan 8, 2016

elide code marked with @boundscheck(...). #14474

Merged

5 tasks

vtjnash mentioned this pull request Nov 8, 2021

simplify inline cost computation, update docs #42997

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

decouple inference from Base #11274

decouple inference from Base #11274

vtjnash commented May 14, 2015

vtjnash May 14, 2015

vtjnash May 15, 2015

timholy commented May 14, 2015

tkelman commented May 14, 2015

tkelman commented May 15, 2015

vtjnash commented May 15, 2015

tkelman commented May 15, 2015

tkelman commented May 15, 2015

carnaval commented May 15, 2015

StefanKarpinski commented May 15, 2015

Keno May 15, 2015

vtjnash May 15, 2015

Keno May 15, 2015

vtjnash May 15, 2015

vtjnash May 16, 2015

Keno May 16, 2015

vtjnash May 16, 2015

Keno May 16, 2015

tkelman May 16, 2015

vtjnash commented May 15, 2015

vtjnash commented May 17, 2015

tkelman commented May 19, 2015

vtjnash commented May 19, 2015

ihnorton commented May 19, 2015

tkelman commented May 19, 2015

ihnorton commented May 20, 2015

sjkelly commented May 20, 2015

ihnorton commented May 20, 2015

tkelman commented May 20, 2015

ihnorton commented May 21, 2015

timholy commented May 29, 2015

tkelman commented May 29, 2015

vtjnash commented May 29, 2015

decouple inference from Base #11274

decouple inference from Base #11274

Conversation

vtjnash commented May 14, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

timholy commented May 14, 2015

tkelman commented May 14, 2015

tkelman commented May 15, 2015

vtjnash commented May 15, 2015

tkelman commented May 15, 2015

tkelman commented May 15, 2015

carnaval commented May 15, 2015

StefanKarpinski commented May 15, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vtjnash commented May 15, 2015

vtjnash commented May 17, 2015

tkelman commented May 19, 2015

vtjnash commented May 19, 2015

ihnorton commented May 19, 2015

tkelman commented May 19, 2015

ihnorton commented May 20, 2015

sjkelly commented May 20, 2015

ihnorton commented May 20, 2015

tkelman commented May 20, 2015

ihnorton commented May 21, 2015

timholy commented May 29, 2015

tkelman commented May 29, 2015

vtjnash commented May 29, 2015