1351804 - Switch libpref over to ArenaAllocator

Status: RESOLVED FIXED

(Core :: Preferences: Backend, enhancement)

Description

[Eric Rahm [:erahm]]

MozReview-Commit-ID: 4GovbBFUBb9

Comment 1

[Eric Rahm [:erahm]]

Attachment: Switch libpref over to ArenaAllocator

Comment 2

[Nathan Froyd [:froydnj]]

Comment on attachment 8852619 [details] [diff] [review]
Switch libpref over to ArenaAllocator

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

::: modules/libpref/prefapi.cpp
@@ +66,5 @@
>      return (strcmp(prefEntry->key, otherKey) == 0);
>  }
>  
>  PLDHashTable*       gHashTable;
> +static ArenaAllocator<8192,4> gPrefNameArena;

I'm not super-excited about the static constructor this is going to add.  Maybe we can make the ArenaAllocator constructor constexpr to avoid that?

Or, alternatively, we could have:

struct GlobalPrefState
{
  // Add the obvious constructor, etc.
  PLDHashTable mHashTable;
  ArenaAllocator<...> mPrefNameArena;
};

static GlobalPrefState* gState;

and use that everywhere?

Comment 3

[Eric Rahm [:erahm]]

(In reply to Nathan Froyd [:froydnj] from comment #2)
> Comment on attachment 8852619 [details] [diff] [review]
> Switch libpref over to ArenaAllocator
> 
> Review of attachment 8852619 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: modules/libpref/prefapi.cpp
> @@ +66,5 @@
> >      return (strcmp(prefEntry->key, otherKey) == 0);
> >  }
> >  
> >  PLDHashTable*       gHashTable;
> > +static ArenaAllocator<8192,4> gPrefNameArena;
> 
> I'm not super-excited about the static constructor this is going to add. 
> Maybe we can make the ArenaAllocator constructor constexpr to avoid that?

I'll make it constexpr in the blocking bug.

> Or, alternatively, we could have:
> 
> struct GlobalPrefState
> {
>   // Add the obvious constructor, etc.
>   PLDHashTable mHashTable;
>   ArenaAllocator<...> mPrefNameArena;
> };
> 
> static GlobalPrefState* gState;
> 
> and use that everywhere?

That sounds like more hassle than it's worth :)

Comment 4

[Eric Rahm [:erahm]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/c788438e745ec9663a9e29ae37a3197995669431
Bug 1351804 - Switch libpref over to ArenaAllocator. r=froydnj

Comment 5

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/c788438e745e