Faster clojure.core/delay implementation

Description

clojure.lang.Delay uses a synchronized lock to protect the deref method, because it must make sure the Delay object is realized exactly once.

As we known synchronized lock plays worse performance under heavy concurrency. Instead, using volatile and double-checking lock in this situation improves the performance. The benchmark code is at test-delay.clj. The benchmark-delay function accepts thread count number as an argument, and it will start as many threads as the argument to access delay object concurrently as in (time (benchmark-delay 1)).

threads

1.9.0-alpha14

+ patch

% better

1

0.570196 ms

0.499905 ms

12

10

19.66194 ms

1.313828 ms

93

20

40.740032 ms

2.149794 ms

95

100

184.041421 ms

8.317295 ms

95

Patch: fast_delay_with_volatile_fn2.patch
Prescreened by: Alex Miller

Environment

macOS Sierra
intel Core i7 2.5G Hz
Memory 16GB

java version "1.8.0_66"
Java(TM) SE Runtime Environment (build 1.8.0_66-b17)
Java HotSpot(TM) 64-Bit Server VM (build 25.66-b17, mixed mode)

Activity

Show:
dennis zane
November 29, 2016, 3:54 PM

The patch which does't mark fn volatile.

dennis zane
November 29, 2016, 3:59 PM

Hi, alex.

I understand your opinion here. Though i still insist that fn doesn't need to be marked as volatile, but it's not a critical concern here.

I uploaded two patches, one is marked fn volatile, the other is not. All of them fixed the whitespace errors and update the benchmark result in ticket description.

Thanks.

dennis zane
November 29, 2016, 4:15 PM

Rebase master.

Nicola Mometto
November 30, 2016, 5:53 PM

dennis, here's an article describing why fn needs to be volatile: https://www.cs.umd.edu/~pugh/java/memoryModel/DoubleCheckedLocking.html

dennis zane
December 1, 2016, 12:01 AM

@Nicola

I knew the double-checking issue in old JVM memory model, but it is different here.
There is no instance constructed, it's only assigning fn to be null, so it doesn't have instruments reordering. And we don't have a partial constructed instance that escapes.

But it's not critical concern here, it seems that volatile doesn't compact performance of this patch.

Thanks.

Completed

Assignee

Unassigned

Reporter

dennis zane

Labels

Approval

Ok

Patch

Code and Test

Fix versions

Affects versions

Priority

Major
Configure