Opened 12 years ago

Last modified 4 years ago

#2028 new bug

STM slightly conservative on write-only transactions

Reported by: JulesBean Owned by:
Priority: lowest Milestone:
Component: Compiler Version: 6.8.1
Keywords: Cc:
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: Runtime performance bug Test Case:
Blocked By: Blocking:
Related Tickets: Differential Rev(s):
Wiki Page:

Description

The STM appears to be slightly too conservative on write-only transactions.

It will store a copy of the current (that is, old) value on a writeTVar, even if it was never read from. This can cause a spurious retry.

E.g. atomically $ writeTVar tv 0 really has no reason to ever retry. Neither, in fact, does atomically $ mapM (\tv -> writeTVar tv 0) [tv1,tv2,tv3,tv4].

My suggestion is to instead indicate "no prior value" for that TVar, and then make no consistency checks on TVars marked as no prior value...

Jules

Change History (21)

comment:1 Changed 12 years ago by simonpj

Comment from Tim Harris:

This suggestion is OK from the point of view of correctness.

There are several special cases where we could allow transactions to commit when we currently treat them as conflicting. E.g. any single-word transaction is OK.

I've avoided dealing with too many of these in the absence of code we care about performing badly (it'll add complexity and might slow down the hopefully-common case of non-conflicting transactions).

No strong arguments against adding this one though: we could probably use a special value in the old-val field to indicate no-previous-value so we won't get a space code. We could also handle it specially in "retry" -- we don't need to watch for updates to a TVar that has been written to but not read.

comment:2 Changed 12 years ago by Isaac Dupree

Probably I don't completely understand STM, but... if these happen at the same time, won't we potentially end up with an inconsistent state if no-one retries?

atomically $ mapM (\tv -> writeTVar tv 0) [tv1,tv2,tv3,tv4]
atomically $ mapM (\tv -> writeTVar tv 1) [tv4,tv3,tv2,tv1]

(and don't single-word transactions depend on the hardware/architecture as for whether they are atomic?)

comment:3 in reply to:  2 Changed 12 years ago by JulesBean

No.

One them has to commit first. When it commits, it sets all the entries to something (say, 1). At this point, the other one has made no visible changes (all its changes are private, in its transaction log). Then, the other one commits, and it sets them all to 0.

Committing is atomic.

In the current situation what would happen is the first would commit, and then the second would say "erk! something has changed!" and not commit.

comment:4 Changed 12 years ago by igloo

Milestone: 6.8 branch
Type: proposalrun-time performance bug

comment:5 Changed 11 years ago by igloo

Milestone: 6.8 branch6.10 branch

comment:6 Changed 11 years ago by simonmar

Architecture: UnknownUnknown/Multiple

comment:7 Changed 11 years ago by simonmar

Operating System: UnknownUnknown/Multiple

comment:8 Changed 10 years ago by igloo

Milestone: 6.10 branch6.12 branch

comment:9 Changed 10 years ago by simonmar

Type of failure: Runtime performance bug

comment:10 Changed 9 years ago by igloo

Milestone: 6.12 branch6.12.3

comment:11 Changed 9 years ago by igloo

Milestone: 6.12.36.14.1
Priority: normallow

comment:12 Changed 9 years ago by igloo

Milestone: 7.0.17.0.2

comment:13 Changed 9 years ago by igloo

Milestone: 7.0.27.2.1

comment:14 Changed 8 years ago by igloo

Milestone: 7.2.17.4.1

comment:15 Changed 8 years ago by igloo

Milestone: 7.4.17.6.1
Priority: lowlowest

comment:16 Changed 7 years ago by igloo

Milestone: 7.6.17.6.2

comment:17 Changed 5 years ago by thoughtpolice

Milestone: 7.6.27.10.1

Moving to 7.10.1.

comment:18 Changed 5 years ago by thoughtpolice

Milestone: 7.10.17.12.1

Moving to 7.12.1 milestone; if you feel this is an error and should be addressed sooner, please move it back to the 7.10.1 milestone.

comment:19 Changed 5 years ago by thoughtpolice

Moving to 7.12.1 milestone; if you feel this is an error and should be addressed sooner, please move it back to the 7.10.1 milestone.

comment:20 Changed 4 years ago by thoughtpolice

Milestone: 7.12.18.0.1

Milestone renamed

comment:21 Changed 4 years ago by thomie

Milestone: 8.0.1
Note: See TracTickets for help on using tickets.