Closed Bug 1163916 Opened 9 years ago Closed 9 years ago

Remove two static constructors from XPCOM

Categories

(Core :: XPCOM, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla41
Tracking Status
firefox41 --- fixed

People

(Reporter: n.nethercote, Assigned: n.nethercote)

Details

Attachments

(2 files)

Some low-hanging fruit, here.
Comment on attachment 8604505 [details] [diff] [review]
(part 1) - Avoid a static nsDebugImpl constructor

Review of attachment 8604505 [details] [diff] [review]:
-----------------------------------------------------------------

I think in this case it might be easier (and less confusing, given that somebody will wonder why |sImpl| isn't a strong reference) to simply default nsDebugImpl's constructor:

class nsDebugImpl ...
{
  nsDebugImpl() = default;
  ...
}

The compiler understands |= default|, whereas even an empty constructor causes static constructors.  (This is true for GCC; I think Clang and MSVC are smart enough to handle both identically.)

r=me with that change.

::: xpcom/base/nsDebugImpl.cpp
@@ -578,5 @@
>  #warning do not know how to break on this platform
>  #endif
>  }
>  
> -static const nsDebugImpl kImpl;

Ah, yes, this is the infamous "static xpcom instance with special AddRef/Release functions".
Attachment #8604505 - Flags: review?(nfroyd) → review+
Oh, I forgot to upload this one yesterday.
Attachment #8604938 - Flags: review?(nfroyd)
Comment on attachment 8604938 [details] [diff] [review]
(part 2) - Avoid a static constructor for kCompressedBufferLength

Review of attachment 8604938 [details] [diff] [review]:
-----------------------------------------------------------------

r=me, your call on whether to change the below.

::: xpcom/io/SnappyUncompressInputStream.cpp
@@ +23,5 @@
> +  if (kCompressedBufferLength == 0) {
> +    kCompressedBufferLength =
> +      detail::SnappyFrameUtils::MaxCompressedBufferLength(snappy::kBlockSize);
> +    MOZ_ASSERT(kCompressedBufferLength > 0);
> +  }

You can just do:

static size_t kCompressedBufferLength = detail::...;

MOZ_ASSERT(kCompressedBufferLength > 0);
return kCompressedBufferLength;

I suppose that needlessly MOZ_ASSERTs, but I don't suspect this code is particularly performance sensitive in tests.  And this way should be ever-so-slightly more efficient in release anyway...
Attachment #8604938 - Flags: review?(nfroyd) → review+
> You can just do:
> 
> static size_t kCompressedBufferLength = detail::...;
> 
> MOZ_ASSERT(kCompressedBufferLength > 0);
> return kCompressedBufferLength;

So if the |static size_t kCompressedBufferLength = ...| is at the top-level it causes a static ctor, but if it's within a function it doesn't? I wouldn't have believed it if count_ctors.py didn't confirm it. How interesting.
glandium explained to me how this works -- |static| variables in a function are initialized upon the first call, not at start-up.

That means I'll be able to avoid some tests in bug 1164373 and maybe some of my other patches. Nice.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: