Re: [gold][patch] Control caching of memory mapped regions

[Ian Lance Taylor <iant at google dot com>]

Cary Coutant <ccoutant@google.com> writes:

>         * fileread.cc (File_read::~File_read): Don't delete whole_file_view_.
>         (File_read::open[1]): Remove initial mapping of whole_file_view_.
>         (File_read::open[2]): Add whole_file_view_ to list of views.
>         (File_read::make_view): Remove test of whole_file_view_.
>         (File_read::find_or_make_view): Create whole_file_view_ if
>         necessary.
>         (File_read::clear_views): Replace bool parameter with enum;
>         adjust all callers.  Don't delete views with unowned data;
>         do delete cached views and views from archives if
>         --no-keep-files-mapped is set.  Set whole_file_view_ to NULL
>         if clearing the corresponding view.
>         * fileread.h (File_read::Clear_views_mode): New enum.
>         (File_read::View::is_data_owned): New method.
>         (File_read::clear_views): Replace bool parameter
>         with enum; adjust all callers.
>         * options.h (General_options): Change keep_files_mapped option;
>         add map_whole_files.
>         * readsyms.cc (Add_symbols::run): Delete sd_ object before
>         releasing the file.
>         * reloc.cc (Scan_relocs::run): Delete rd_ object before releasing
>         the file.

>  void
> -File_read::clear_views(bool destroying)
> +File_read::clear_views(Clear_views_mode mode)
>  {
> +  bool keep_files_mapped = (parameters->options_valid()
> +			    && parameters->options().keep_files_mapped());
>    Views::iterator p = this->views_.begin();
>    while (p != this->views_.end())
>      {
>        bool should_delete;
> -      if (p->second->is_locked())
> +      if (p->second->is_locked() || !p->second->is_data_owned())
>  	should_delete = false;
> -      else if (destroying)
> +      else if (mode == CLEAR_VIEWS_ALL)
>  	should_delete = true;
> -      else if (p->second->should_cache())
> +      else if (p->second->should_cache() && keep_files_mapped)
>  	should_delete = false;
> -      else if (this->object_count_ > 1 && p->second->accessed())
> +      else if (this->object_count_ > 1
> +      	       && p->second->accessed()
> +      	       && mode != CLEAR_VIEWS_ARCHIVE)
>  	should_delete = false;
>        else
>  	should_delete = true;

I'm having a little trouble understanding the test of is_data_owned().
is_data_owned() will return true if the data_ownership_ field is not
DATA_NOT_OWNED.  The only time that field will be DATA_NOT_OWNED is
for the whole file view created for an in-memory file, which is only
done for testing.  So in normal usage is_data_owned() will always
return true.  Given the tests above, that seems to suggest that
should_delete is pretty much always going to be false.  That is
obviously not what your results show, so what am I missing?

Everything else looks fine.

Ian