use ScopedValues for MPFR precision and rounding by simonbyrne · Pull Request #51362 · JuliaLang/julia

simonbyrne · 2023-09-17T21:10:58Z

Should fix thread safety issues.

Actually fixes #27139

longemen3000 · 2023-09-17T22:31:17Z

Could this fix #51058?

vchuravy · 2023-09-18T01:45:43Z

base/mpfr.jl

+const CURRENT_ROUNDING_MODE = ScopedValue{MPFRRoundingMode}()
 const DEFAULT_PRECISION = Ref{Clong}(256)
-
+const CURRENT_PRECISION = ScopedValue{Clong}()


You could set it to default precision? Or is it important that the default precision could be changed globally?

We support it at the moment, so dropping it would be breaking, but I'm not sure how necessary it is?

If you need to be able to change it, don't you need this to be

const CURRENT_PRECISION = ScopedValue{RefValue{Clong}}(Ref{Clong}())

since ScopedValue is immutable?

No, but the default field is marked const:

julia/base/scopedvalues.jl

Lines 40 to 46 in b189bed

mutable struct ScopedValue{T}

const has_default::Bool

const default::T

ScopedValue{T}() where T = new(false)

ScopedValue{T}(val) where T = new{T}(true, val)

ScopedValue(val::T) where T = new{T}(true, val)

end

I didn't mean that it's an immutable struct, I just mean that you can't actually change the value other than by introducing a new dynamic scope.

Yes, that's correct.

vchuravy · 2023-09-18T01:46:41Z

Nice! I would be curious to see what's the performance impact.

simonbyrne · 2023-09-18T16:04:51Z

base/mpfr.jl

 significand_limb_count(x::BigFloat) = div(sizeof(x._d), sizeof(Limb), RoundToZero)

-rounding_raw(::Type{BigFloat}) = ROUNDING_MODE[]
+rounding_raw(::Type{BigFloat}) = something(Base.ScopedValues.get(CURRENT_ROUNDING_MODE), ROUNDING_MODE[])


@vchuravy should this overload Base.get?

It has different API so I decided not to.

get(dict, key, value) vs get(ScopedValue). But that was me being conservative, also it returns Some/Nothing.

I could see an argument for get(SValue, value)

simonbyrne · 2023-09-19T22:43:15Z

I see a minor impact:

function f(N)
    x = big(0.0)
    for i = 1:N
        x += BigFloat("0.1")
    end
    return x
end

function g(N)
    setrounding(BigFloat, RoundDown) do
        f(N)
    end
end

On master:

julia> @btime f(1_000_000)
  197.769 ms (5000002 allocations: 202.18 MiB)
100000.0000000000000000000000000000000000000000000000000000000000000000001391156

julia> @btime g(1_000_000)
  202.435 ms (5000002 allocations: 202.18 MiB)
99999.99999999999999999999999999999999999999999999999999999999999999999955990192

On this branch:

julia> @btime f(1_000_000)
  208.775 ms (5000002 allocations: 202.18 MiB)
100000.0000000000000000000000000000000000000000000000000000000000000000001391156

julia> @btime g(1_000_000)
  251.740 ms (5000007 allocations: 202.18 MiB)
99999.99999999999999999999999999999999999999999999999999999999999999999955990192

So a bit of overhead, but not too bad.

simonbyrne · 2023-09-20T22:20:57Z

To clarify, this would access the scoped values around 4 million times (in each iteration, access both rounding mode and precision for thee decimal to binary conversion, then the addition), so 40 milliseconds of overhead would be about 10ns per call.

simonbyrne · 2023-09-21T17:11:17Z

Could this fix #51058?

No.

vchuravy · 2023-09-21T17:36:28Z

Changing the precision has a bit more overhead, but we would assume that's a rare operation to begin with?

simonbyrne · 2023-09-21T17:43:12Z

Changing the precision has a bit more overhead, but we would assume that's a rare operation to begin with?

Actually, that's probably more common. However I see roughly the same overhead when using the following:

function h(N)
    setprecision(BigFloat, 256) do
        f(N)
    end
end

KristofferC · 2024-02-13T12:47:21Z

What's the status here @simonbyrne? Rebase and merge?

simonbyrne · 2024-02-25T14:36:20Z

I think this is ready to go now.

Should fix thread safety issues. Actually fixes JuliaLang#27139

nsajko · 2024-10-01T03:00:40Z

PR #55911 fixes a regression introduced by this PR, could anyone help review it?

Another follow-up to JuliaLang#51362. Make sure the global default precision and rounding mode are only dereferenced when necessary (when there's no relevant scope, in the ScopedValues sense). Currently this change doesn't result in performance improvements, presumably due to how costly the access to a `ScopedValue` currently is, but the idea is to avoid the cost of the dereference when possible. Once ScopedValues are better optimized by the compiler, I guess this would also result in better effects in case it's known that a call is within a ScopedValues scope.

nsajko · 2025-01-24T09:54:36Z

Tried cherry-picking on top of v1.11.3 and it seems to apply cleanly. Marking for backport.

simonbyrne · 2025-01-24T15:25:51Z

I'm not sure this is a great candidate for backport, as it does change behavior slightly

simonbyrne · 2025-01-24T15:26:19Z

There is also an open bug for scoped values #56062 that may cause problems

nsajko · 2025-01-24T16:14:06Z

You're right. There's also this regression, not fixed yet: #55899.

Another follow-up to #51362. Make sure the global default precision and rounding mode are only dereferenced when necessary (when there's no relevant scope, in the ScopedValues sense). Currently this change doesn't result in performance improvements, presumably due to how costly the access to a `ScopedValue` currently is, but the idea is to avoid the cost of the dereference when possible. Once ScopedValues are better optimized by the compiler, I guess this would also result in better effects in case it's known that a call is within a ScopedValues scope.

simonbyrne requested a review from vchuravy September 17, 2023 21:11

brenhinkeller added bugfix This change fixes an existing bug bignums BigInt and BigFloat labels Sep 17, 2023

vchuravy reviewed Sep 18, 2023

View reviewed changes

simonbyrne commented Sep 18, 2023

View reviewed changes

simonbyrne marked this pull request as ready for review September 21, 2023 05:03

simonbyrne mentioned this pull request Sep 23, 2023

Interactive scopes #51434

Open

Joel-Dahne mentioned this pull request Oct 25, 2023

Possibly use ScopedValues for precision (and rounding?) kalmarek/Arblib.jl#171

Closed

OlivierHnt mentioned this pull request Jan 11, 2024

Non-thread safe use of setrounding JuliaIntervals/IntervalArithmetic.jl#612

Closed

use ScopedValues for MPFR precision and rounding

f9dc27f

simonbyrne force-pushed the sb/mpfr-scope branch from 9aee915 to f9dc27f Compare February 25, 2024 05:37

vchuravy approved these changes Feb 26, 2024

View reviewed changes

KristofferC merged commit 3ebba8f into master Feb 26, 2024

KristofferC deleted the sb/mpfr-scope branch February 26, 2024 14:06

tecosaur pushed a commit to tecosaur/julia that referenced this pull request Mar 4, 2024

use ScopedValues for MPFR precision and rounding (JuliaLang#51362)

6ee01b0

Should fix thread safety issues. Actually fixes JuliaLang#27139

mkitti pushed a commit to mkitti/julia that referenced this pull request Mar 7, 2024

use ScopedValues for MPFR precision and rounding (JuliaLang#51362)

d9ca7cd

Should fix thread safety issues. Actually fixes JuliaLang#27139

Zentrik mentioned this pull request Mar 7, 2024

Performance Regression for BigFloat due to #51362 #53641

Closed

devmotion mentioned this pull request Apr 16, 2024

Fix setprecision errors on Julia nightly kalmarek/Arblib.jl#187

Merged

This was referenced Sep 27, 2024

How to handle the precision of BigFloats #10040

Closed

setprecision(::Function, ::Type, ::Int) regression #55899

Closed

devmotion mentioned this pull request Oct 9, 2024

Fix tests and update with recent changes in Base JuliaMath/IrrationalConstants.jl#36

Merged

nsajko mentioned this pull request Oct 10, 2024

Base.MPFR: avoid reading the global defaults unless necessary #56095

Merged

vtjnash mentioned this pull request Jan 22, 2025

Make ScopedValues.get a method of Base.get #54553

Open

nsajko mentioned this pull request Jan 24, 2025

Comparison with irrationals is not thread safe #52862

Closed

nsajko added the backport 1.11 Change should be backported to release-1.11 label Jan 24, 2025

nsajko removed the backport 1.11 Change should be backported to release-1.11 label Jan 24, 2025

Seelengrab mentioned this pull request May 31, 2025

restore fallback 3-arg setprecision method #58586

Merged

arnavk23 mentioned this pull request Jun 3, 2025

Backport: Add thread-safety note to NEWS.md in v1.12 #58615

Closed

	mutable struct ScopedValue{T}
	const has_default::Bool
	const default::T
	ScopedValue{T}() where T = new(false)
	ScopedValue{T}(val) where T = new{T}(true, val)
	ScopedValue(val::T) where T = new{T}(true, val)
	end

Uh oh!

Conversation

simonbyrne commented Sep 17, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

longemen3000 commented Sep 17, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vchuravy commented Sep 18, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

simonbyrne commented Sep 19, 2023

Uh oh!

simonbyrne commented Sep 20, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

simonbyrne commented Sep 21, 2023

Uh oh!

vchuravy commented Sep 21, 2023

Uh oh!

simonbyrne commented Sep 21, 2023

Uh oh!

KristofferC commented Feb 13, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

simonbyrne commented Feb 25, 2024

Uh oh!

nsajko commented Oct 1, 2024

Uh oh!

nsajko commented Jan 24, 2025

Uh oh!

simonbyrne commented Jan 24, 2025

Uh oh!

simonbyrne commented Jan 24, 2025

Uh oh!

nsajko commented Jan 24, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

simonbyrne commented Sep 17, 2023 •

edited

Loading

simonbyrne commented Sep 20, 2023 •

edited

Loading

KristofferC commented Feb 13, 2024 •

edited

Loading