Page MenuHomeSoftware Heritage
Differential D5369

HgLoaderFromDisk: Don't query revision_get with release ids
ClosedPublic
Actions

Authored by vlorentz on Mar 29 2021, 2:09 PM.

Details

Reviewers
Alphare
ardumont
Group Reviewers
Reviewers
Commits
rDLDHG428d764c403c: HgLoaderFromDisk: Don't query revision_get with release ids
Summary

This is a minor performance optimization, removing items from the
call to revision_get when we know their result will be None.

Motivation: A future commit (T3140) will refactor this function,
and dealing only with revision ids makes it simpler.

Diff Detail

Event Timeline

vlorentz created this revision.Mar 29 2021, 2:09 PM
vlorentz added a child revision: D5371: HgLoaderFromDisk: Read and write ExtIDs to find revisions already loaded.
swh-public-ci added a comment.Mar 29 2021, 2:11 PM

Build is green

Patch application report for D5369 (id=19242)

Rebasing onto a62318d725...

Current branch diff-target is up to date.
Changes applied before test
commit 69f4b023b3ecf21e23838a46935069245dcfd118
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Mon Mar 29 12:34:36 2021 +0200

    HgLoaderFromDisk: Don't query revision_get with release ids
    
    This is a minor performance optimization, removing items from the
    call to revision_get when we know their result will be None.
    
    Motivation: A future commit will refactor this function, and dealing only
    with revision ids makes it simpler.

See https://jenkins.softwareheritage.org/job/DLDHG/job/tests-on-diff/187/ for more details.

Harbormaster completed remote builds in B20313: Diff 19242.Mar 29 2021, 2:11 PM
vlorentz requested review of this revision.Mar 29 2021, 2:11 PM
ardumont accepted this revision.Mar 29 2021, 5:03 PM
ardumont added a subscriber: ardumont.

lgtm

and, right, I did not realize...

.oO(wondering if we do not reproduce this elsewhere now...)

This revision is now accepted and ready to land.Mar 29 2021, 5:03 PM
Alphare accepted this revision.Mar 29 2021, 5:52 PM
vlorentz updated this revision to Diff 19261.Mar 30 2021, 9:42 AM

rebase

swh-public-ci added a comment.Mar 30 2021, 9:44 AM

Build is green

Patch application report for D5369 (id=19261)

Rebasing onto 020791c464...

Current branch diff-target is up to date.
Changes applied before test
commit 428d764c403c4d69c802e989bc5cf001b614921e
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Mon Mar 29 12:34:36 2021 +0200

    HgLoaderFromDisk: Don't query revision_get with release ids
    
    This is a minor performance optimization, removing items from the
    call to revision_get when we know their result will be None.
    
    Motivation: A future commit will refactor this function, and dealing only
    with revision ids makes it simpler.

See https://jenkins.softwareheritage.org/job/DLDHG/job/tests-on-diff/194/ for more details.

Harbormaster completed remote builds in B20329: Diff 19261.Mar 30 2021, 9:44 AM
Closed by commit rDLDHG428d764c403c: HgLoaderFromDisk: Don't query revision_get with release ids (authored by vlorentz). · Explain WhyMar 30 2021, 9:46 AM
This revision was automatically updated to reflect the committed changes.
vlorentz added a commit: rDLDHG428d764c403c: HgLoaderFromDisk: Don't query revision_get with release ids.

Revision Contents
Changeset List

PathSizeCoverage (All)Coverage (Touched)
swh/
loader/
mercurial/
4 lines92%
Loading...

Diff 19242

View Options

swh/loader/mercurial/from_disk.py

# Copyright (C) 2015-2021 The Software Heritage developers # Copyright (C) 2015-2021 The Software Heritage developers
# See the AUTHORS file at the top-level directory of this distribution # See the AUTHORS file at the top-level directory of this distribution
# License: GNU General Public License version 3, or any later version # License: GNU General Public License version 3, or any later version
# See top-level LICENSE file for more information # See top-level LICENSE file for more information
from __future__ import annotations from __future__ import annotations
import binascii import binascii
import datetime import datetime
import enum import enum
from functools import lru_cache from functools import lru_cache
import hashlib import hashlib
import re import re
from typing import Any, Dict, Iterable, List, Optional, Tuple, Union from typing import (
Any,
Dict,
Generic,
Iterable,
List,
Optional,
Tuple,
Type,
TypeVar,
Union,
)
import warnings import warnings
import attr import attr
from attrs_strict import type_validator from attrs_strict import type_validator
from .collections import ImmutableDict from .collections import ImmutableDict
from .exceptions import ValidationError from .exceptions import ValidationError
from .fields.hashes import validate_sha1 from .fields.hashes import validate_sha1
▲ Show 20 Lines • Show All 707 Lines • ▼ Show 20 Lines_swhid_type_map = {
"rel": RELEASE, "rel": RELEASE,
"rev": REVISION, "rev": REVISION,
"dir": DIRECTORY, "dir": DIRECTORY,
"cnt": CONTENT, "cnt": CONTENT,
"emd": RAW_EXTRINSIC_METADATA, "emd": RAW_EXTRINSIC_METADATA,
} }
@attr.s(frozen=True, kw_only=True) # type of the "object_type" attribute of the SWHID class; either
class CoreSWHID: # ObjectType or ExtendedObjectType
""" _TObjectType = TypeVar("_TObjectType", ObjectType, ExtendedObjectType)
Dataclass holding the relevant info associated to a SoftWare Heritage
persistent IDentifier (SWHID). # the SWHID class itself (this is used so that X.from_string() can return X
# for all X subclass of _BaseSWHID)
Unlike `QualifiedSWHID`, it is restricted to core SWHIDs, ie. SWHIDs _TSWHID = TypeVar("_TSWHID", bound="_BaseSWHID")
with no qualifiers.
Raises:
swh.model.exceptions.ValidationError: In case of invalid object type or id
To get the raw SWHID string from an instance of this class,
use the :func:`str` function:
>>> swhid = CoreSWHID(
... object_type=ObjectType.CONTENT,
... object_id=bytes.fromhex('8ff44f081d43176474b267de5451f2c2e88089d0'),
... )
>>> str(swhid)
'swh:1:cnt:8ff44f081d43176474b267de5451f2c2e88089d0'
And vice-versa with :meth:`CoreSWHID.from_string`: @attr.s(frozen=True, kw_only=True)
class _BaseSWHID(Generic[_TObjectType]):
"""Common base class for CoreSWHID, QualifiedSWHID, and ExtendedSWHID.
>>> swhid == CoreSWHID.from_string( This is an "abstract" class and should not be instantiated directly;
... "swh:1:cnt:8ff44f081d43176474b267de5451f2c2e88089d0" it only exists to deduplicate code between these three SWHID classes."""
... )
True
"""
namespace = attr.ib(type=str, default=SWHID_NAMESPACE) namespace = attr.ib(type=str, default=SWHID_NAMESPACE)
"""the namespace of the identifier, defaults to ``swh``""" """the namespace of the identifier, defaults to ``swh``"""
scheme_version = attr.ib(type=int, default=SWHID_VERSION) scheme_version = attr.ib(type=int, default=SWHID_VERSION)
"""the scheme version of the identifier, defaults to 1""" """the scheme version of the identifier, defaults to 1"""
object_type = attr.ib( # overridden by subclasses
type=ObjectType, validator=type_validator(), converter=ObjectType object_type: _TObjectType
)
"""the type of object the identifier points to""" """the type of object the identifier points to"""
object_id = attr.ib(type=bytes, validator=type_validator()) object_id = attr.ib(type=bytes, validator=type_validator())
"""object's identifier""" """object's identifier"""
@namespace.validator @namespace.validator
def check_namespace(self, attribute, value): def check_namespace(self, attribute, value):
if value != SWHID_NAMESPACE: if value != SWHID_NAMESPACE:
Show All 23 Lines def __str__(self) -> str:
self.namespace, self.namespace,
str(self.scheme_version), str(self.scheme_version),
self.object_type.value, self.object_type.value,
hash_to_hex(self.object_id), hash_to_hex(self.object_id),
] ]
) )
@classmethod @classmethod
def from_string(cls, s: str) -> CoreSWHID: def from_string(cls: Type[_TSWHID], s: str) -> _TSWHID:
parts = _parse_swhid(s) parts = _parse_swhid(s)
if parts.pop("qualifiers"): if parts.pop("qualifiers"):
olasdUnsubmitted
Not Done
Inline Actions
parts = _parse_swhid(s)
- if parts.pop("qualifiers"):
+ if "qualifiers" in parts:
raise ValidationError(f"{cls.__name__} does not support qualifiers.")
olasd:
vlorentzAuthorUnsubmitted
Done
Inline Actions

Nope, parts["qualifiers"] is an empty dict

vlorentz: Nope, `parts["qualifiers"]` is an empty dict
raise ValidationError("CoreSWHID does not support qualifiers.") raise ValidationError(f"{cls.__name__} does not support qualifiers.")
try: try:
return CoreSWHID(**parts) return cls(**parts)
except ValueError as e: except ValueError as e:
raise ValidationError(*e.args) from None raise ValidationError(*e.args) from None
@attr.s(frozen=True, kw_only=True)
class CoreSWHID(_BaseSWHID[ObjectType]):
"""
Dataclass holding the relevant info associated to a SoftWare Heritage
persistent IDentifier (SWHID).
Unlike `QualifiedSWHID`, it is restricted to core SWHIDs, ie. SWHIDs
with no qualifiers.
Raises:
swh.model.exceptions.ValidationError: In case of invalid object type or id
To get the raw SWHID string from an instance of this class,
use the :func:`str` function:
>>> swhid = CoreSWHID(
... object_type=ObjectType.CONTENT,
... object_id=bytes.fromhex('8ff44f081d43176474b267de5451f2c2e88089d0'),
... )
>>> str(swhid)
'swh:1:cnt:8ff44f081d43176474b267de5451f2c2e88089d0'
And vice-versa with :meth:`CoreSWHID.from_string`:
>>> swhid == CoreSWHID.from_string(
... "swh:1:cnt:8ff44f081d43176474b267de5451f2c2e88089d0"
... )
True
"""
object_type = attr.ib(
type=ObjectType, validator=type_validator(), converter=ObjectType
)
"""the type of object the identifier points to"""
def _parse_core_swhid(swhid: Union[str, CoreSWHID, None]) -> Optional[CoreSWHID]: def _parse_core_swhid(swhid: Union[str, CoreSWHID, None]) -> Optional[CoreSWHID]:
"""Alias of CoreSWHID.from_string to make mypy happy...... """Alias of CoreSWHID.from_string to make mypy happy......
https://github.com/python/mypy/issues/6172""" https://github.com/python/mypy/issues/6172"""
if swhid is None or isinstance(swhid, CoreSWHID): if swhid is None or isinstance(swhid, CoreSWHID):
return swhid return swhid
else: else:
return CoreSWHID.from_string(swhid) return CoreSWHID.from_string(swhid)
def _parse_lines_qualifier( def _parse_lines_qualifier(
lines: Union[str, Tuple[int, Optional[int]], None] lines: Union[str, Tuple[int, Optional[int]], None]
) -> Optional[Tuple[int, Optional[int]]]: ) -> Optional[Tuple[int, Optional[int]]]:
if lines is None or isinstance(lines, tuple): if lines is None or isinstance(lines, tuple):
return lines return lines
elif "-" in lines: elif "-" in lines:
return tuple(map(int, lines.split("-", 2))) # type: ignore return tuple(map(int, lines.split("-", 2))) # type: ignore
else: else:
return (int(lines), None) return (int(lines), None)
@attr.s(frozen=True, kw_only=True) @attr.s(frozen=True, kw_only=True)
class QualifiedSWHID: class QualifiedSWHID(_BaseSWHID[ObjectType]):
""" """
Dataclass holding the relevant info associated to a SoftWare Heritage Dataclass holding the relevant info associated to a SoftWare Heritage
persistent IDentifier (SWHID) persistent IDentifier (SWHID)
Raises: Raises:
swh.model.exceptions.ValidationError: In case of invalid object type or id swh.model.exceptions.ValidationError: In case of invalid object type or id
To get the raw SWHID string from an instance of this class, To get the raw SWHID string from an instance of this class,
Show All 10 Linesclass QualifiedSWHID(_BaseSWHID[ObjectType]):
And vice-versa with :meth:`QualifiedSWHID.from_string`: And vice-versa with :meth:`QualifiedSWHID.from_string`:
>>> swhid == QualifiedSWHID.from_string( >>> swhid == QualifiedSWHID.from_string(
... "swh:1:cnt:8ff44f081d43176474b267de5451f2c2e88089d0;lines=5-10" ... "swh:1:cnt:8ff44f081d43176474b267de5451f2c2e88089d0;lines=5-10"
... ) ... )
True True
""" """
namespace = attr.ib(type=str, default=SWHID_NAMESPACE)
"""the namespace of the identifier, defaults to ``swh``"""
scheme_version = attr.ib(type=int, default=SWHID_VERSION)
"""the scheme version of the identifier, defaults to 1"""
object_type = attr.ib( object_type = attr.ib(
type=ObjectType, validator=type_validator(), converter=ObjectType type=ObjectType, validator=type_validator(), converter=ObjectType
) )
"""the type of object the identifier points to""" """the type of object the identifier points to"""
object_id = attr.ib(type=bytes, validator=type_validator())
"""object's identifier"""
# qualifiers: # qualifiers:
origin = attr.ib(type=Optional[str], default=None, validator=type_validator()) origin = attr.ib(type=Optional[str], default=None, validator=type_validator())
"""the software origin where an object has been found or observed in the wild, """the software origin where an object has been found or observed in the wild,
as an URI""" as an URI"""
visit = attr.ib(type=Optional[CoreSWHID], default=None, converter=_parse_core_swhid) visit = attr.ib(type=Optional[CoreSWHID], default=None, converter=_parse_core_swhid)
"""the core identifier of a snapshot corresponding to a specific visit """the core identifier of a snapshot corresponding to a specific visit
Show All 19 Linesclass QualifiedSWHID(_BaseSWHID[ObjectType]):
lines = attr.ib( lines = attr.ib(
type=Optional[Tuple[int, Optional[int]]], type=Optional[Tuple[int, Optional[int]]],
default=None, default=None,
validator=type_validator(), validator=type_validator(),
converter=_parse_lines_qualifier, converter=_parse_lines_qualifier,
) )
"""lines: line number(s) of interest, usually within a content object""" """lines: line number(s) of interest, usually within a content object"""
@namespace.validator
def check_namespace(self, attribute, value):
if value != SWHID_NAMESPACE:
raise ValidationError(
"Invalid SWHID: invalid namespace: %(namespace)s",
params={"namespace": value},
)
@scheme_version.validator
def check_scheme_version(self, attribute, value):
if value != SWHID_VERSION:
raise ValidationError(
"Invalid SWHID: invalid version: %(version)s", params={"version": value}
)
@object_id.validator
def check_object_id(self, attribute, value):
if len(value) != 20:
raise ValidationError(
"Invalid SWHID: invalid checksum: %(object_id)s",
params={"object_id": hash_to_hex(value)},
)
@visit.validator @visit.validator
def check_visit(self, attribute, value): def check_visit(self, attribute, value):
if value and value.object_type != ObjectType.SNAPSHOT: if value and value.object_type != ObjectType.SNAPSHOT:
raise ValidationError( raise ValidationError(
f"The 'visit' qualifier must be a 'snp' SWHID, " f"The 'visit' qualifier must be a 'snp' SWHID, "
f"not '{value.object_type.value}'" f"not '{value.object_type.value}'"
) )
▲ Show 20 Lines • Show All 46 Lines • ▼ Show 20 Lines def from_string(cls, s: str) -> QualifiedSWHID:
) )
try: try:
return QualifiedSWHID(**parts, **qualifiers) return QualifiedSWHID(**parts, **qualifiers)
except ValueError as e: except ValueError as e:
raise ValidationError(*e.args) from None raise ValidationError(*e.args) from None
@attr.s(frozen=True, kw_only=True) @attr.s(frozen=True, kw_only=True)
class ExtendedSWHID: class ExtendedSWHID(_BaseSWHID[ExtendedObjectType]):
""" """
Dataclass holding the relevant info associated to a SoftWare Heritage Dataclass holding the relevant info associated to a SoftWare Heritage
persistent IDentifier (SWHID). persistent IDentifier (SWHID).
It extends `CoreSWHID`, by allowing non-standard object types; and should It extends `CoreSWHID`, by allowing non-standard object types; and should
only be used internally to Software Heritage. only be used internally to Software Heritage.
Raises: Raises:
Show All 12 Linesclass ExtendedSWHID(_BaseSWHID[ExtendedObjectType]):
And vice-versa with :meth:`CoreSWHID.from_string`: And vice-versa with :meth:`CoreSWHID.from_string`:
>>> swhid == ExtendedSWHID.from_string( >>> swhid == ExtendedSWHID.from_string(
... "swh:1:cnt:8ff44f081d43176474b267de5451f2c2e88089d0" ... "swh:1:cnt:8ff44f081d43176474b267de5451f2c2e88089d0"
... ) ... )
True True
""" """
namespace = attr.ib(type=str, default=SWHID_NAMESPACE)
"""the namespace of the identifier, defaults to ``swh``"""
scheme_version = attr.ib(type=int, default=SWHID_VERSION)
"""the scheme version of the identifier, defaults to 1"""
object_type = attr.ib( object_type = attr.ib(
type=ExtendedObjectType, type=ExtendedObjectType,
validator=type_validator(), validator=type_validator(),
converter=ExtendedObjectType, converter=ExtendedObjectType,
) )
"""the type of object the identifier points to""" """the type of object the identifier points to"""
object_id = attr.ib(type=bytes, validator=type_validator())
"""object's identifier"""
@namespace.validator
def check_namespace(self, attribute, value):
if value != SWHID_NAMESPACE:
raise ValidationError(
"Invalid SWHID: invalid namespace: %(namespace)s",
params={"namespace": value},
)
@scheme_version.validator
def check_scheme_version(self, attribute, value):
if value != SWHID_VERSION:
raise ValidationError(
"Invalid SWHID: invalid version: %(version)s", params={"version": value}
)
@object_id.validator
def check_object_id(self, attribute, value):
if len(value) != 20:
raise ValidationError(
"Invalid SWHID: invalid checksum: %(object_id)s",
params={"object_id": hash_to_hex(value)},
)
def __str__(self) -> str:
return SWHID_SEP.join(
[
self.namespace,
str(self.scheme_version),
self.object_type.value,
hash_to_hex(self.object_id),
]
)
@classmethod
def from_string(cls, s: str) -> ExtendedSWHID:
parts = _parse_swhid(s)
if parts.pop("qualifiers"):
raise ValidationError("ExtendedSWHID does not support qualifiers.")
return ExtendedSWHID(**parts)
@attr.s(frozen=True) @attr.s(frozen=True)
class SWHID: class SWHID:
""" """
Deprecated alternative to QualifiedSWHID. Deprecated alternative to QualifiedSWHID.
Args: Args:
namespace (str): the namespace of the identifier, defaults to ``swh`` namespace (str): the namespace of the identifier, defaults to ``swh``
▲ Show 20 Lines • Show All 196 Lines • Show Last 20 Lines
About · Developer information · Legal