1473135 - Stop using ReentrantMonitor in StringBundle classes

Status: RESOLVED FIXED

(Core :: Internationalization, enhancement, P3)

Description

[Kris Maglione [:kmag]]

ReentrantMonitors are major overkill for what string bundles are doing. They're never used as monitors, and the sections where they are held are guaranteed to never be re-entrant. We should be using simple mutexes instead, since they use significantly less memory and are significantly more performant.

Comment 1

[Kris Maglione [:kmag]]

Attachment: Bug 1473135: Stop using ReentrantMonitor for string bundle mutexes.

Review commit: https://reviewboard.mozilla.org/r/258536/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/258536/

Comment 2

[Zibi Braniecki [:zbraniecki][:gandalf]]

Comment on attachment 8993864 [details]
Bug 1473135: Stop using ReentrantMonitor for string bundle mutexes.

https://reviewboard.mozilla.org/r/258536/#review265576

Hmm, the patch looks good, but I'm surprised to hear that Monitor is noticably heavier than Mutex.

Is there any documentation on mozilla's monitor/mutex implementations and when which one should be used?

Comment 3

[Kris Maglione [:kmag]]

(In reply to Zibi Braniecki [:gandalf][:zibi] from comment #2)
> Hmm, the patch looks good, but I'm surprised to hear that Monitor is
> noticably heavier than Mutex.

As a rule, every monitor uses at least one mutex, in addition to a cond, which is at least as heavy as a mutex plus the wait list it has to maintain.

Re-entrant monitors are generally even worse, but ours are especially bad. They just wrap PRMonitor, which contains a mutex, two conds, an owner thread, a name pointer, and a bunch of accounting data:

https://searchfox.org/mozilla-central/rev/ad36eff63e208b37bc9441b91b7cea7291d82890/nsprpub/pr/include/private/primpl.h#1448-1483

They're something like 224 bytes a piece, not including any private data that libc allocates for them.

> Is there any documentation on mozilla's monitor/mutex implementations and
> when which one should be used?

Unfortunately, no. The general rule is to just always use the lightest one you can get away with.

Comment 4

[Kris Maglione [:kmag]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/9ba70b8ff0f755d5575333a71267805e0ece02f6
Bug 1473135: Stop using ReentrantMonitor for string bundle mutexes. r=gandalf

Comment 5

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/9ba70b8ff0f7