Split TemplateHashMapImpl::Lookup into two methods

test/cctest/test-api.cc

 // Copyright 2012 the V8 project authors. All rights reserved.
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
 // met:
 //
 //     * Redistributions of source code must retain the above copyright
 //       notice, this list of conditions and the following disclaimer.
 //     * Redistributions in binary form must reproduce the above
 //       copyright notice, this list of conditions and the following
 //       disclaimer in the documentation and/or other materials provided
@@ skipping 12334 matching lines @@
   CHECK(jitcode_line_info != NULL);
 
   class DummyJitCodeLineInfo {
   };
 
   switch (event->type) {
     case v8::JitCodeEvent::CODE_ADDED: {
         CHECK(event->code_start != NULL);
         CHECK_NE(0, static_cast<int>(event->code_len));
         CHECK(event->name.str != NULL);
-        i::HashMap::Entry* entry =
-            code_map->Lookup(event->code_start,
-                             i::ComputePointerHash(event->code_start),
-                             true);
+        i::HashMap::Entry* entry = code_map->LookupOrInsert(
+            event->code_start, i::ComputePointerHash(event->code_start));
         entry->value = reinterpret_cast<void*>(event->code_len);
 
         if (FunctionNameIs("bar", event)) {
           ++saw_bar;
         }
       }
       break;
 
     case v8::JitCodeEvent::CODE_MOVED: {
         uint32_t hash = i::ComputePointerHash(event->code_start);
         // We would like to never see code move that we haven't seen before,
         // but the code creation event does not happen until the line endings
         // have been calculated (this is so that we can report the line in the
         // script at which the function source is found, see
         // Compiler::RecordFunctionCompilation) and the line endings
         // calculations can cause a GC, which can move the newly created code
         // before its existence can be logged.
-        i::HashMap::Entry* entry =
-            code_map->Lookup(event->code_start, hash, false);
+        i::HashMap::Entry* entry = code_map->Lookup(event->code_start, hash);
         if (entry != NULL) {
           ++move_events;
 
           CHECK_EQ(reinterpret_cast<void*>(event->code_len), entry->value);
           code_map->Remove(event->code_start, hash);
 
-          entry = code_map->Lookup(event->new_code_start,
-                                   i::ComputePointerHash(event->new_code_start),
-                                   true);
-          CHECK(entry != NULL);

[marja, 2015/04/10 07:59:58]

Why did you remove this CHECK?

[adamk, 2015/04/13 18:34:41]

Basically because it's testing internal invariants of the HashMap
implementation, which doesn't seem appropriate for this test (we have HashMap
tests for that). I've left similar DCHECKs around in production code, though,
which maybe I should remove too. But this one struck me as particularly wrong
(and harmless to remove).

+          entry = code_map->LookupOrInsert(
+              event->new_code_start,
+              i::ComputePointerHash(event->new_code_start));
           entry->value = reinterpret_cast<void*>(event->code_len);
         }
       }
       break;
 
     case v8::JitCodeEvent::CODE_REMOVED:
       // Object/code removal events are currently not dispatched from the GC.
       CHECK(false);
       break;
 
     // For CODE_START_LINE_INFO_RECORDING event, we will create one
     // DummyJitCodeLineInfo data structure pointed by event->user_dat. We
     // record it in jitcode_line_info.
     case v8::JitCodeEvent::CODE_START_LINE_INFO_RECORDING: {
         DummyJitCodeLineInfo* line_info = new DummyJitCodeLineInfo();
         v8::JitCodeEvent* temp_event = const_cast<v8::JitCodeEvent*>(event);
         temp_event->user_data = line_info;
-        i::HashMap::Entry* entry =
-            jitcode_line_info->Lookup(line_info,
-                                      i::ComputePointerHash(line_info),
-                                      true);
+        i::HashMap::Entry* entry = jitcode_line_info->LookupOrInsert(
+            line_info, i::ComputePointerHash(line_info));
         entry->value = reinterpret_cast<void*>(line_info);
       }
       break;
     // For these two events, we will check whether the event->user_data
     // data structure is created before during CODE_START_LINE_INFO_RECORDING
     // event. And delete it in CODE_END_LINE_INFO_RECORDING event handling.
     case v8::JitCodeEvent::CODE_END_LINE_INFO_RECORDING: {
         CHECK(event->user_data != NULL);
         uint32_t hash = i::ComputePointerHash(event->user_data);
         i::HashMap::Entry* entry =
-            jitcode_line_info->Lookup(event->user_data, hash, false);
+            jitcode_line_info->Lookup(event->user_data, hash);
         CHECK(entry != NULL);
         delete reinterpret_cast<DummyJitCodeLineInfo*>(event->user_data);
       }
       break;
 
     case v8::JitCodeEvent::CODE_ADD_LINE_POS_INFO: {
         CHECK(event->user_data != NULL);
         uint32_t hash = i::ComputePointerHash(event->user_data);
         i::HashMap::Entry* entry =
-            jitcode_line_info->Lookup(event->user_data, hash, false);
+            jitcode_line_info->Lookup(event->user_data, hash);
         CHECK(entry != NULL);
       }
       break;
 
     default:
       // Impossible event.
       CHECK(false);
       break;
   }
 }
@@ skipping 9453 matching lines @@
   }
   {
     v8::TryCatch try_catch;
     uint16_t* data = reinterpret_cast<uint16_t*>(buffer);
     CHECK(v8::String::NewFromTwoByte(isolate, data, v8::String::kNormalString,
                                      length).IsEmpty());
     CHECK(!try_catch.HasCaught());
   }
   free(buffer);
 }