Page MenuHomeSoftware Heritage
Differential D6424 Diff 24097 swh/perfecthash/__init__.py

Changeset View

Standalone View

View Options

swh/perfecthash/__init__.py

# Copyright (C) 2021 The Software Heritage developers
ardumontUnsubmitted
Done
Inline Actions
+ # Copyright (C) 2021 The Software Heritage developers
+ # See the AUTHORS file at the top-level directory of this distribution
+ # License: GNU General Public License version 3, or any later version
+ # See top-level LICENSE file for more information
+
from _hash_cffi import lib
from cffi import FFI

copyright headers to match other swh modules?

ardumont: copyright headers to match other swh modules?
dacharyAuthorUnsubmitted
Done
Inline Actions

Good point, done!

dachary: Good point, done!
# See the AUTHORS file at the top-level directory of this distribution
# License: GNU General Public License version 3, or any later version
# See top-level LICENSE file for more information
from typing import NewType
from cffi import FFI
from swh.perfecthash._hash_cffi import lib
Key = NewType("Key", bytes)
HashObject = NewType("HashObject", bytes)
class Shard:
"""Low level management for files indexed with a perfect hash table.
ardumontUnsubmitted
Done
Inline Actions
and looking up the content of an object when given the key.
"""
- def __init__(self, path):
+ def __init__(self, path: str):
"""Initialize with an existing Read Shard.
Args:
- path (str): path to an existing Read Shard file or device
+ path: path to an existing Read Shard file or device
"""
self.ffi = FFI()

Let's add some types.

ardumont: Let's add some types.
This class allows creating a Read Shard by adding key/object pairs
and looking up the content of an object when given the key.
"""
def __init__(self, path: str):
"""Initialize with an existing Read Shard.
Args:
path: path to an existing Read Shard file or device
"""
self.ffi = FFI()
self.shard = lib.shard_init(path.encode("utf-8"))
ardumontUnsubmitted
Done
Inline Actions
lib.shard_destroy(self.shard)
- def create(self, objects_count):
+ def create(self, objects_count: int) -> "Shard":
"""Wipe out the content of the Read Shard. It must be followed by
**object_count** calls to the **write** method otherwise the content
of the Read Shard will be inconsistent. When all objects are inserted,
the Read Shard must be made persistent by calling the **save** method.
Args:
- objects_count (int): number of objects in the Read Shard.
+ objects_count: number of objects in the Read Shard.
Returns:

fyi, we now are documenting directly the types within the code and no longer in the docstring.

ardumont: fyi, we now are documenting directly the types within the code and no longer in the docstring.
def __del__(self):
lib.shard_destroy(self.shard)
def create(self, objects_count: int) -> "Shard":
"""Wipe out the content of the Read Shard. It must be followed by
**object_count** calls to the **write** method otherwise the content
of the Read Shard will be inconsistent. When all objects are inserted,
the Read Shard must be made persistent by calling the **save** method.
Args:
objects_count: number of objects in the Read Shard.
Returns:
self.
"""
assert lib.shard_create(self.shard, objects_count) != -1
return self
ardumontUnsubmitted
Done
Inline Actions
return self
- def load(self):
+ def load(self) -> "Shard":
"""Open the Read Shard file in read-only mode.

From the docstring.

ardumont: From the docstring.
def load(self) -> "Shard":
"""Open the Read Shard file in read-only mode.
Returns:
self.
"""
assert lib.shard_load(self.shard) != -1
return self
ardumontUnsubmitted
Done
Inline Actions
return self
- def save(self):
+ def save(self) -> int:
"""Create the perfect hash table the **lookup** method

From the docstring ;)

ardumont: From the docstring ;)
def save(self) -> int:
"""Create the perfect hash table the **lookup** method
relies on to find the content of the objects.
It must be called after **create** an **write** otherwise the
content of the Read Shard will be inconsistent.
Returns:
0 on success, -1 on error.
"""
return lib.shard_save(self.shard)
def lookup(self, key: Key) -> HashObject:
ardumontUnsubmitted
Done
Inline Actions

indent is off by 2 chars almost everywhere.
We normalize on a 4 chars indent below "Args:" and "Returns:" (and "Raises:" if any).

ardumont: indent is off by 2 chars almost everywhere. We normalize on a 4 chars indent below "Args:" and…
ardumontUnsubmitted
Done
Inline Actions
return lib.shard_save(self.shard)
- def lookup(self, key):
+ def lookup(self, key: Key) -> HashObject:
"""Fetch the object matching the key in the Read Shard.
Fetching an object is O(1): one lookup in the index to obtain
the offset of the object in the Read Shard and one read to get
the payload.
Args:
- key (32 bytes): the key associated with the object to retrieve.
+ key: the key associated with the object to retrieve.
Returns:
- the object as bytes.
+ the object as bytes.
"""
object_pointer = self.ffi.new("char**")

Let's add some types around this, then define at the top-level module something like:

from typing import NewType

Key = NewType("Key", bytes)
HashObjects = NewType("HashObject", bytes)

I believe that improves the reading significantly.

ardumont: Let's add some types around this, then define at the top-level module something like: ``` from…
"""Fetch the object matching the key in the Read Shard.
Fetching an object is O(1): one lookup in the index to obtain
the offset of the object in the Read Shard and one read to get
the payload.
Args:
key: the key associated with the object to retrieve.
Returns:
the object as bytes.
"""
object_size_pointer = self.ffi.new("uint64_t*")
lib.shard_lookup_object_size(self.shard, key, object_size_pointer)
object_size = object_size_pointer[0]
object_pointer = self.ffi.new("char[]", object_size)
lib.shard_lookup_object(self.shard, object_pointer, object_size)
ardumontUnsubmitted
Done
Inline Actions

Something hit me, can't the lookup fail?

What would be the return if that was the case None?
(if so, the type must be changed to Optional[HashObject]).

ardumont: Something hit me, can't the lookup fail? What would be the return if that was the case None?
dacharyAuthorUnsubmitted
Done
Inline Actions

I overlooked this question, sorry. The lookup cannot fail: if given a key that does not exist it will return a random object. Which is ok because the global index ensures the Read Shard is only queried with objects that are known to exist in the hash map.

dachary: I overlooked this question, sorry. The lookup cannot fail: if given a key that does not exist…
ardumontUnsubmitted
Done
Inline Actions

you did not overlook it, it's a new one!

If anything, that's me who overlooked that code last time ;)

Great! "Can't happen!"

ardumont: you did not overlook it, it's a new one! If anything, that's me who overlooked that code last…
return self.ffi.buffer(object_pointer, object_size)
ardumontUnsubmitted
Done
Inline Actions
return self.ffi.buffer(object_pointer[0], object_size_pointer[0])
- def write(self, key, object):
+ def write(self, key: Key, object: HashObject) -> int:
"""Add the key/object pair to the Read Shard.
The **create** method must have been called prior to calling
the **write** method.
Args:
- key (32 bytes): the unique key associated with the object.
- object (bytes): the object
+ key: the unique key associated with the object.
+ object: the object
Returns:
- 0 on success, -1 on error.
+ 0 on success, -1 on error.
"""
return lib.shard_object_write(self.shard, key, object, len(object))
ardumont:
def write(self, key: Key, object: HashObject) -> int:
"""Add the key/object pair to the Read Shard.
The **create** method must have been called prior to calling
the **write** method.
Args:
key: the unique key associated with the object.
object: the object
Returns:
0 on success, -1 on error.
"""
return lib.shard_object_write(self.shard, key, object, len(object))
About · Developer information · Legal