Skip to content

Refactor: Rename BlobReader to BlobFinder #818

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
olaotesile wants to merge 4 commits into from
+9 −8
Closed

Refactor: Rename BlobReader to BlobFinder #818

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
733bbdd
Refactor: Rename BlobReader to BlobFinder
olaotesile Dec 26, 2025
5579abb
Merge remote-tracking branch 'upstream/dev'
olaotesile Jan 2, 2026
b997904
Restored original filenames. kept BlobReader to BlobFinder class rename
olaotesile Jan 6, 2026
a0bb7b5
Merge branch 'dev' into main
olaotesile Jan 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions io/blob_store.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -412,8 +412,8 @@ class BlobStore {
std::vector<hwy::uint128_t> directory_; // two per blob, see `SetRange`.
}; // BlobStore

BlobReader::BlobReader(const Path& blob_path) : blob_path_(blob_path) {
PROFILER_ZONE("Startup.BlobReader");
BlobFinder::BlobFinder(const Path& blob_path) : blob_path_(blob_path) {
PROFILER_ZONE("Startup.BlobFinder");

file_ = OpenFileOrAbort(blob_path, "r");
file_bytes_ = file_->FileSize();
Expand Down
8 changes: 4 additions & 4 deletions io/blob_store.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,13 +46,13 @@ struct BlobRange {

// Reads `BlobStore` header, converts keys to strings and creates a hash map for
// faster lookups.
// TODO(janwas): rename to BlobFinder or similar.
// Finds and manages the location of data blobs within the store.
// Thread-safe: it is safe to concurrently call all methods except `CloseFile`.
class BlobReader {
class BlobFinder {
public:
// Acquires ownership of `file` (which must be non-null) and reads its header.
// Aborts on error.
explicit BlobReader(const Path& blob_path);
explicit BlobFinder(const Path& blob_path);

const Path& blob_path() const { return blob_path_; }

Expand Down Expand Up @@ -136,4 +136,4 @@ class BlobWriter {

} // namespace gcpp

#endif // THIRD_PARTY_GEMMA_CPP_IO_BLOB_STORE_H_
#endif // THIRD_PARTY_GEMMA_CPP_IO_BLOB_STORE_H_
5 changes: 3 additions & 2 deletions io/blob_store_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ namespace {
class BlobStoreTest : public testing::Test {};
#endif

// Change the first argument from BlobFinderTest to BlobStoreTest
Copy link

@mudkipdev mudkipdev Jan 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this a comment that an AI left?

Copy link

@Ataskmaster Ataskmaster Jan 12, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This guy used AI just to rename ONE (1) class. Honestly if you guys merge this in you are just invitng more trouble in future from people following this guy's approach just so they can put "core contibutor @google/deepmind" in their bio. Also how is he raising PR's here without even having basic understanding of bazel.

Copy link

@ThePangel ThePangel Jan 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, but it will be setting a precedent that lazy, AI PRs built on zero understanding of the actual code will be merged, for what? To ruin the code slowly PR by PR creating bigger problems in the future just so one guy can say "I work at Google deep mind"? (PR OPs actual statement)

Copy link

@princeflexzy0 princeflexzy0 Jan 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know why he would bloody lie unprovoked lol 😂

TEST(BlobStoreTest, TestReadWrite) {
ThreadingArgs threading_args;
ThreadingContext ctx(threading_args);
Expand All @@ -59,7 +60,7 @@ TEST(BlobStoreTest, TestReadWrite) {

std::fill(buffer.begin(), buffer.end(), 0);

const BlobReader reader(path);
const BlobFinder reader(path);

HWY_ASSERT_EQ(reader.Keys().size(), 2);
HWY_ASSERT_STRING_EQ(reader.Keys()[0].c_str(), keyA.c_str());
Expand Down Expand Up @@ -126,7 +127,7 @@ TEST(BlobStoreTest, TestNumBlobs) {
HWY_ASSERT(blobs.size() == num_blobs);
writer.Finalize();

BlobReader reader(path);
BlobFinder reader(path);
HWY_ASSERT_EQ(reader.Keys().size(), num_blobs);

ParallelFor(
Expand Down
Loading