Skip to content

Commit

Permalink
Reland of remove uses of folly::hash::fnv32_buf (#40233)
Browse files Browse the repository at this point in the history
Summary:
Pull Request resolved: #40233

changelog: [internal]

This is a reland of D49355595 and D49358327.

the problem was using two different hashing functions in place where they must be the same.

Reviewed By: javache

Differential Revision: D50020135

fbshipit-source-id: 1cec6bc385077d371b024a0fb5d9c64ba1f6269c
  • Loading branch information
sammy-SC authored and facebook-github-bot committed Oct 9, 2023
1 parent 6a2c245 commit 67384cf
Show file tree
Hide file tree
Showing 5 changed files with 68 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include <react/renderer/core/EventLogger.h>
#include "NativePerformanceObserver.h"

#include <functional>
#include <unordered_map>

namespace facebook::react {
Expand Down Expand Up @@ -298,8 +299,7 @@ void PerformanceEntryReporter::scheduleFlushBuffer() {

struct StrKey {
uint32_t key;
StrKey(std::string_view s)
: key(folly::hash::fnv32_buf(s.data(), s.length())) {}
StrKey(std::string_view s) : key(std::hash<std::string_view>{}(s)) {}

bool operator==(const StrKey& rhs) const {
return key == rhs.key;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
#pragma once

#include <react/renderer/core/RawPropsPrimitives.h>
#include <react/utils/fnv1a.h>
#include <functional>

// We need to use clang pragmas inside of a macro below,
// so we need to pull out the "if" statement here.
Expand All @@ -22,11 +24,11 @@
([]() constexpr->RawPropsPropNameHash { \
CLANG_PRAGMA("clang diagnostic push") \
CLANG_PRAGMA("clang diagnostic ignored \"-Wshadow\"") \
return folly::hash::fnv32_buf(s, sizeof(s) - 1); \
return RAW_PROPS_KEY_HASH(s); \
CLANG_PRAGMA("clang diagnostic pop") \
}())

#define RAW_PROPS_KEY_HASH(s) folly::hash::fnv32_buf(s, std::strlen(s))
#define RAW_PROPS_KEY_HASH(s) facebook::react::fnv1a(s)

// Convenience for building setProps switch statements.
// This injects `fromRawValue` into source; each file that uses
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ void RawPropsParser::iterateOverValues(

auto name = nameValue.utf8(runtime);

auto nameHash = RAW_PROPS_KEY_HASH(name.c_str());
auto nameHash = RAW_PROPS_KEY_HASH(name);
auto rawValue = RawValue(jsi::dynamicFromValue(runtime, value));

visit(nameHash, name.c_str(), rawValue);
Expand All @@ -213,7 +213,7 @@ void RawPropsParser::iterateOverValues(
for (const auto& pair : dynamic.items()) {
auto name = pair.first.getString();

auto nameHash = RAW_PROPS_KEY_HASH(name.c_str());
auto nameHash = RAW_PROPS_KEY_HASH(name);
auto rawValue = RawValue{pair.second};
visit(nameHash, name.c_str(), rawValue);
}
Expand Down
36 changes: 36 additions & 0 deletions packages/react-native/ReactCommon/react/utils/fnv1a.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
/*
* Copyright (c) Meta Platforms, Inc. and affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

#pragma once

namespace facebook::react {

/**
* FNV-1a hash function implementation.
* Implemented as described in http://www.isthe.com/chongo/tech/comp/fnv/.
*
* Please use std::hash if possible. `fnv1a` should only be used in cases
* when std::hash does not provide the needed functionality. For example,
* constexpr.
*/
constexpr uint32_t fnv1a(std::string_view string) noexcept {
constexpr uint32_t offset_basis = 2166136261;

uint32_t hash = offset_basis;

for (auto const& c : string) {
hash ^= static_cast<int8_t>(c);
// Using shifts and adds instead of multiplication with a prime number.
// This is faster when compiled with optimizations.
hash +=
(hash << 1) + (hash << 4) + (hash << 7) + (hash << 8) + (hash << 24);
}

return hash;
}

} // namespace facebook::react
24 changes: 24 additions & 0 deletions packages/react-native/ReactCommon/react/utils/tests/fnv1aTests.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
/*
* Copyright (c) Meta Platforms, Inc. and affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

#include <gtest/gtest.h>
#include <react/utils/fnv1a.h>

namespace facebook::react {

TEST(fnv1aTests, testBasicHashing) {
EXPECT_EQ(fnv1a("react"), fnv1a("react"));

EXPECT_NE(fnv1a("react"), fnv1a("tceat"));

auto string1 = "case 1";
auto string2 = "different string";
EXPECT_EQ(fnv1a(string1), fnv1a(string1));
EXPECT_NE(fnv1a(string1), fnv1a(string2));
}

} // namespace facebook::react

0 comments on commit 67384cf

Please sign in to comment.