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

treat globals as constant within top-level expressions when possible #524

Closed
pao opened this issue Mar 5, 2012 · 18 comments
Closed

treat globals as constant within top-level expressions when possible #524

pao opened this issue Mar 5, 2012 · 18 comments
Assignees
Milestone

Comments

@pao
Copy link
Member

pao commented Mar 5, 2012

   _       _ _(_)_     |
  (_)     | (_) (_)    |  A fresh approach to technical computing
   _ _   _| |_  __ _   |
  | | | | | | |/ _` |  |  Version 0.0.0-prerelease
  | | |_| | | | (_| |  |  Commit 335d1ef401 (2012-03-04 23:47:29)
 _/ |\__'_|_|_|\__'_|  |
|__/                   |

julia> x = 5
5

julia> typeof(x)
Int64

julia> typeof([i | i = 1:5])
Array{Int64,1}

So far, so good.

julia> typeof([i | i = 1:x])
Array{Any,1}

Wat.
(with apologies to https://www.destroyallsoftware.com/talks/wat)

@JeffBezanson
Copy link
Member

This behavior is actually expected at the moment. Since x is global, it might change anywhere so we can't assume we know its type. This is overly pessimistic, but it's hard to come up with a rule that would let us do better.

@pao
Copy link
Member Author

pao commented Mar 5, 2012

It's type would have to be known when it is used in the listcomp, though, as it is when I call typeof(x) directly?

@nolta
Copy link
Member

nolta commented Mar 5, 2012

Ok, so:

julia> test(n) = [i|i=1:n]

julia> typeof(test(5))
Array{Int64,1}

Speaking of global vs function scope, i got tripped up by this one the other day:

julia> a = 3
3

julia> @eval a + 1
4

julia> function f(n)
       b = n
       @eval b + 1
       end

julia> f(3)
b not defined

Silly nolta, eval is for globals.

@StefanKarpinski
Copy link
Member

Ah, yes. Julia's eval always evaluates in the top-level scope.

@JeffBezanson
Copy link
Member

I tried to send the following by email but it didn't seem to work:

Yes, you and I can see that the type of x doesn't change before it's used by the comprehension, but this is hard for the compiler to reason about. It doesn't yet have knowledge about which functions modify globals. We could "localize" globals inside inputs so that only assignments in the repl input itself can modify repl global variables.

@pao
Copy link
Member Author

pao commented Mar 7, 2012

Okay, that makes sense. Perhaps we can write up some documentation on these global effects (potential) pitfalls, then? I can add that to my to-do list. (Is there a "documentation" tag?)

@JeffBezanson
Copy link
Member

Well, as I said, we could decide to make the repl effectively a "local" scope, and assume that its variables can't be modified elsewhere.
REPL semantics are always troublesome.

@StefanKarpinski
Copy link
Member

One way to handle this would be to make each repl expression behave as though all globals where let-bound when evaluating the expression, thereby making them behave like locals while evaluating, and then copy the local value back to the global at the end. Effectively this is like saying that nothing else can change globals while the repl expression is running. Which seems reasonable.

@pao
Copy link
Member Author

pao commented Mar 9, 2012

Wandering further down the listcomp rabbit hole:

julia> typeof([a | a in 1:10 ])
Array{Int64,1}

julia> a = 'a'
'a'

julia> typeof([a | a in 1:10 ])
Array{Any,1}

Without some way to descope variables (MATLAB clear, for instance) and/or Stefan's suggestion and/or creating gensyms for loop variables such as a above to keep them from getting captured, long running REPL sessions will eventually end in some kind of death by type entropy. I personally am a fan of putting "and" into the previous sentence.

@StefanKarpinski
Copy link
Member

Leading to the type-entropy death of the universe? Grim.

@waldyrious
Copy link
Contributor

Related: #964

@StefanKarpinski
Copy link
Member

Renamed to "slow global scope" to reflect the problem, not a particular potential solution.

@StefanKarpinski
Copy link
Member

At this point I think the best solution may be to solve this together with #265: track what generated code depends on the type of a global, and invalidate that code if the global's type is changed. This solves the common-case performance issue. It introduces another potential problem: if you keep changing the type of a global variables, it may cause repeated invalidation of generated code. Perhaps the assumed type of a global should be the typejoin of the previously assumed type and the type of the new value. By explicitly annotating a global with a type (#964), you are both constraining its type and indicating that slow generic code should be generated that can handle any such type. Thus myglobal = 1 would cause code to be generated assuming that myglobal is an Int, whereas myglobal::Integer = 1 would cause code to be generated assuming only that myglobal is an Integer. In the former case, since myglobal is non-constant, you can do myglobal = 1.5 and then code would be generated assuming that myglobal has type typejoin(Int,Float64) i.e. Real. If later someone does myglobal = "foo", then code would be invalidated again, and regenerated assuming that myglobal is of type typejoin(Real,ASCIIString) i.e. Any. After that the code would never be invalidated again since it would be completely generic. It would also be slow, but I think that's warranted in that case.

@timholy
Copy link
Member

timholy commented Jul 23, 2014

Nice idea. Just following up on your mailing-list posts: I wouldn't let the global-scope problem rise to a higher priority than, say, granular package precompilation. using PackageThatUsesLotsOfOtherPackages is a form of slowness that's much more important to overcome.

@StefanKarpinski
Copy link
Member

That's important too but I do think this plus #265 is starting to be a priority.

@ivarne
Copy link
Member

ivarne commented Jul 23, 2014

It is also possible to assume that globals are "type constant" only until a type instability is detected. If a global changes its type once, (it would seem like) it is much more likely to change again, and the slow code we currently generate seems appropriate.

Static analysis might also be able to determine at compile time that a global is type unstable and avoid generating specialized code that might invalidated.

@IainNZ
Copy link
Member

IainNZ commented Jul 23, 2014

Getting OT, but I'm with @timholy as far as priorities go, the package loading time thing is getting to be really really painful...

@StefanKarpinski
Copy link
Member

We're not going to do this exact thing and #8870 covers the broader issue of global performance without presuming a particular solution.

dkarrasch pushed a commit that referenced this issue May 8, 2024
Stdlib: SparseArrays
URL: https://github.com/JuliaSparse/SparseArrays.jl.git
Stdlib branch: main
Julia branch: master
Old commit: cb602d7
New commit: a09f90b
Julia version: 1.12.0-DEV
SparseArrays version: 1.12.0
Bump invoked by: @dkarrasch
Powered by:
[BumpStdlibs.jl](https://github.com/JuliaLang/BumpStdlibs.jl)

Diff:
JuliaSparse/SparseArrays.jl@cb602d7...a09f90b

```
$ git log --oneline cb602d7..a09f90b
a09f90b Adjust matvec and matmatmul! to new internal LinAlg interface (#519)
3b30333 ci: run aqua test as a standalone ci job (#537)
df0a154 Add versioned Manifest files to .gitignore (#534)
4606755 Extend `copytrito!` for a sparse source (#533)
33fbc75 SparseMatrixCSC constructor with a Tuple of Integers (#523)
08d6ae1 CI: don't run `threads` tests in Windows GHA CI (attempt 2) (#530)
7408e4b Revert "Don't fail CI if codecov upload fails." (#527)
287e406 Bump julia-actions/setup-julia from 1 to 2 (#524)
b5de0da Don't fail CI if codecov upload fails. (#525)
78dde4c cast to Float64 directly instead of using float (#521)
a5e95ec CI: Add Apple Silicon (macOS aarch64) to the CI matrix (#505)
```

Co-authored-by: Dilum Aluthge <[email protected]>
xlxs4 pushed a commit to xlxs4/julia that referenced this issue May 9, 2024
…aLang#54406)

Stdlib: SparseArrays
URL: https://github.com/JuliaSparse/SparseArrays.jl.git
Stdlib branch: main
Julia branch: master
Old commit: cb602d7
New commit: a09f90b
Julia version: 1.12.0-DEV
SparseArrays version: 1.12.0
Bump invoked by: @dkarrasch
Powered by:
[BumpStdlibs.jl](https://github.com/JuliaLang/BumpStdlibs.jl)

Diff:
JuliaSparse/SparseArrays.jl@cb602d7...a09f90b

```
$ git log --oneline cb602d7..a09f90b
a09f90b Adjust matvec and matmatmul! to new internal LinAlg interface (JuliaLang#519)
3b30333 ci: run aqua test as a standalone ci job (JuliaLang#537)
df0a154 Add versioned Manifest files to .gitignore (JuliaLang#534)
4606755 Extend `copytrito!` for a sparse source (JuliaLang#533)
33fbc75 SparseMatrixCSC constructor with a Tuple of Integers (JuliaLang#523)
08d6ae1 CI: don't run `threads` tests in Windows GHA CI (attempt 2) (JuliaLang#530)
7408e4b Revert "Don't fail CI if codecov upload fails." (JuliaLang#527)
287e406 Bump julia-actions/setup-julia from 1 to 2 (JuliaLang#524)
b5de0da Don't fail CI if codecov upload fails. (JuliaLang#525)
78dde4c cast to Float64 directly instead of using float (JuliaLang#521)
a5e95ec CI: Add Apple Silicon (macOS aarch64) to the CI matrix (JuliaLang#505)
```

Co-authored-by: Dilum Aluthge <[email protected]>
lazarusA pushed a commit to lazarusA/julia that referenced this issue Jul 12, 2024
…aLang#54406)

Stdlib: SparseArrays
URL: https://github.com/JuliaSparse/SparseArrays.jl.git
Stdlib branch: main
Julia branch: master
Old commit: cb602d7
New commit: a09f90b
Julia version: 1.12.0-DEV
SparseArrays version: 1.12.0
Bump invoked by: @dkarrasch
Powered by:
[BumpStdlibs.jl](https://github.com/JuliaLang/BumpStdlibs.jl)

Diff:
JuliaSparse/SparseArrays.jl@cb602d7...a09f90b

```
$ git log --oneline cb602d7..a09f90b
a09f90b Adjust matvec and matmatmul! to new internal LinAlg interface (JuliaLang#519)
3b30333 ci: run aqua test as a standalone ci job (JuliaLang#537)
df0a154 Add versioned Manifest files to .gitignore (JuliaLang#534)
4606755 Extend `copytrito!` for a sparse source (JuliaLang#533)
33fbc75 SparseMatrixCSC constructor with a Tuple of Integers (JuliaLang#523)
08d6ae1 CI: don't run `threads` tests in Windows GHA CI (attempt 2) (JuliaLang#530)
7408e4b Revert "Don't fail CI if codecov upload fails." (JuliaLang#527)
287e406 Bump julia-actions/setup-julia from 1 to 2 (JuliaLang#524)
b5de0da Don't fail CI if codecov upload fails. (JuliaLang#525)
78dde4c cast to Float64 directly instead of using float (JuliaLang#521)
a5e95ec CI: Add Apple Silicon (macOS aarch64) to the CI matrix (JuliaLang#505)
```

Co-authored-by: Dilum Aluthge <[email protected]>
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

No branches or pull requests

9 participants