From 21b8949d238aef5dd205d81e0d26f6df15afbb3b Mon Sep 17 00:00:00 2001
From: Drew Paroski <andrewparoski@fb.com>
Date: Mon, 29 Apr 2013 23:23:28 -0700
Subject: [PATCH] Make array_key_exists support collections efficiently

array_key_exists() was converting collections to arrays before checking if
a key exists, which is an O(n) operation (which also has the annoying side
effect of changing int-like string keys to int keys.)

This diff makes array_key_exists natively support collections (which also
addresses the pesky int-like string key -> int key issue).
---
 hphp/runtime/ext/ext_array.cpp                |  6 +++++-
 hphp/runtime/vm/translator/hopt/codegen.cpp   |  7 +++++++
 hphp/runtime/vm/translator/translator-x64.cpp |  6 ++++++
 hphp/test/slow/collection_classes/841.php     | 20 +++++++++++++++++++
 .../slow/collection_classes/841.php.expect    | 13 ++++++++++++
 5 files changed, 51 insertions(+), 1 deletion(-)
 create mode 100644 hphp/test/slow/collection_classes/841.php
 create mode 100644 hphp/test/slow/collection_classes/841.php.expect

diff --git a/hphp/runtime/ext/ext_array.cpp b/hphp/runtime/ext/ext_array.cpp
index e4d150253..9b49bf20b 100644
--- a/hphp/runtime/ext/ext_array.cpp
+++ b/hphp/runtime/ext/ext_array.cpp
@@ -194,10 +194,14 @@ bool f_array_key_exists(CVarRef key, CVarRef search) {
   if (LIKELY(saccType == KindOfArray)) {
     ad = Variant::GetArrayData(sacc);
   } else if (saccType == KindOfObject) {
+    ObjectData* obj = Variant::GetObjectData(sacc);
+    if (obj->isCollection()) {
+      return collectionOffsetContains(obj, key);
+    }
     return f_array_key_exists(key, toArray(search));
   } else {
     throw_bad_type_exception("array_key_exists expects an array or an object; "
-                           "false returned.");
+                             "false returned.");
     return false;
   }
   Variant::TypedValueAccessor kacc = key.getTypedAccessor();
diff --git a/hphp/runtime/vm/translator/hopt/codegen.cpp b/hphp/runtime/vm/translator/hopt/codegen.cpp
index 79641c93b..885ae4f44 100644
--- a/hphp/runtime/vm/translator/hopt/codegen.cpp
+++ b/hphp/runtime/vm/translator/hopt/codegen.cpp
@@ -30,6 +30,7 @@
 #include "runtime/base/types.h"
 #include "runtime/ext/ext_closure.h"
 #include "runtime/ext/ext_continuation.h"
+#include "runtime/ext/ext_collections.h"
 #include "runtime/vm/bytecode.h"
 #include "runtime/vm/runtime.h"
 #include "runtime/base/stats.h"
@@ -4547,6 +4548,9 @@ static int64_t ak_exist_int(int64_t key, ArrayData* arr) {
 
 HOT_FUNC_VM
 static int64_t ak_exist_string_obj(StringData* key, ObjectData* obj) {
+  if (obj->isCollection()) {
+    return collectionOffsetContains(obj, key);
+  }
   CArrRef arr = obj->o_toArray();
   int64_t res = ak_exist_string_helper(key, arr.get());
   return res;
@@ -4554,6 +4558,9 @@ static int64_t ak_exist_string_obj(StringData* key, ObjectData* obj) {
 
 HOT_FUNC_VM
 static int64_t ak_exist_int_obj(int64_t key, ObjectData* obj) {
+  if (obj->isCollection()) {
+    return collectionOffsetContains(obj, key);
+  }
   CArrRef arr = obj->o_toArray();
   bool res = arr.get()->exists(key);
   return res;
diff --git a/hphp/runtime/vm/translator/translator-x64.cpp b/hphp/runtime/vm/translator/translator-x64.cpp
index c3ec1ccb9..59b61db4d 100644
--- a/hphp/runtime/vm/translator/translator-x64.cpp
+++ b/hphp/runtime/vm/translator/translator-x64.cpp
@@ -8215,6 +8215,9 @@ static int64_t ak_exist_int(int64_t key, ArrayData* arr) {
 }
 
 static int64_t ak_exist_string_obj(StringData* key, ObjectData* obj) {
+  if (obj->isCollection()) {
+    return collectionOffsetContains(obj, key);
+  }
   CArrRef arr = obj->o_toArray();
   int64_t res = ak_exist_string_helper(key, arr.get());
   decRefObj(obj);
@@ -8223,6 +8226,9 @@ static int64_t ak_exist_string_obj(StringData* key, ObjectData* obj) {
 }
 
 static int64_t ak_exist_int_obj(int64_t key, ObjectData* obj) {
+  if (obj->isCollection()) {
+    return collectionOffsetContains(obj, key);
+  }
   CArrRef arr = obj->o_toArray();
   bool res = arr.get()->exists(key);
   decRefObj(obj);
diff --git a/hphp/test/slow/collection_classes/841.php b/hphp/test/slow/collection_classes/841.php
new file mode 100644
index 000000000..1e951b7c7
--- /dev/null
+++ b/hphp/test/slow/collection_classes/841.php
@@ -0,0 +1,20 @@
+<?php
+function f() {
+  $m = Map {1 => 'a', '2' => 'b'};
+  var_dump(array_key_exists(1, $m));
+  var_dump(array_key_exists('1', $m));
+  var_dump(array_key_exists(2, $m));
+  var_dump(array_key_exists('2', $m));
+  var_dump(array_key_exists(3, $m));
+  var_dump(array_key_exists('3', $m));
+  echo "========\n";
+  $x = 'array_key_exists';
+  $x[0] = 'a';
+  var_dump($x(1, $m));
+  var_dump($x('1', $m));
+  var_dump($x(2, $m));
+  var_dump($x('2', $m));
+  var_dump($x(3, $m));
+  var_dump($x('3', $m));
+}
+f();
diff --git a/hphp/test/slow/collection_classes/841.php.expect b/hphp/test/slow/collection_classes/841.php.expect
new file mode 100644
index 000000000..42de964ac
--- /dev/null
+++ b/hphp/test/slow/collection_classes/841.php.expect
@@ -0,0 +1,13 @@
+bool(true)
+bool(false)
+bool(false)
+bool(true)
+bool(false)
+bool(false)
+========
+bool(true)
+bool(false)
+bool(false)
+bool(true)
+bool(false)
+bool(false)