Github user SolidWallOfCode commented on a diff in the pull request:

    https://github.com/apache/trafficserver/pull/240#discussion_r33810635
  
    --- Diff: iocore/hostdb/P_HostDBProcessor.h ---
    @@ -190,6 +190,14 @@ extern RecRawStatBlock *hostdb_rsb;
     #define HOSTDB_DECREMENT_THREAD_DYN_STAT(_s, _t) 
RecIncrRawStatSum(hostdb_rsb, _t, (int)_s, -1);
     
     
    +struct CmpStrCaseInsensitive {
    +  bool operator()(const std::string &a, const std::string &b) const { 
return strcasecmp(a.c_str(), b.c_str()) == 0; }
    +};
    +// A to hold a ref-counted map
    +struct RefCountedHostsFileMap : public RefCountObj {
    +  std::map<std::string, IpAddr, CmpStrCaseInsensitive> hosts_file_map;
    +};
    --- End diff --
    
    I'd do this differently. You have to read in the file. If you look at the 
original code that file buffer is kept for the lifetime of the use of the hosts 
names and the actual "strings" that are the host names are just pointers in to 
that buffer. This is a bit memory inefficient because you keep the address 
strings in memory but involves many fewer allocations because you don't have to 
copy or allocate for the strings and the memory management is easy since it's 
all one unit.
    
    In that case you 'd use `ts::ConstBuffer` as your string type and use an 
appropriate comparison functor.
    
    This also makes searching faster as you can use ts::ConstBuffer from the 
input name trivially and the lifetime of that is sufficient for the search. 
Look at PR 229, in `LogField.cc` around line 122, for an example that does 
exactly this.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

Reply via email to