Skip to content
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

Jump to bottom

Fix #868: Recoups some of performance losses from PR #841 #869

Merged
merged 4 commits into from
Jun 22, 2023
Merged

Fix #868: Recoups some of performance losses from PR #841 #869

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
00dd1d2
fixed lockReg shared_ptr
tcormackMW Jun 14, 2023
de57c64
down to 5% increase from original
tcormackMW Jun 14, 2023
716ef80
get logs from github to verify behavior
tcormackMW Jun 15, 2023
2ec2075
reverting performance yml and adding move constructor (default) for R…
tcormackMW Jun 16, 2023
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
2 changes: 1 addition & 1 deletion framework/include/cppmicroservices/ServiceRegistrationBase.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ namespace cppmicroservices

ServiceRegistrationBase(BundlePrivate* bundle, InterfaceMapConstPtr const& service, Properties&& props);

std::shared_ptr<ServiceRegistrationLocks> LockServiceRegistration() const;
ServiceRegistrationLocks LockServiceRegistration() const;

std::shared_ptr<ServiceRegistrationBasePrivate> d;
};
Expand Down
4 changes: 2 additions & 2 deletions framework/src/service/ServiceReferenceBasePrivate.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,10 +54,10 @@ namespace cppmicroservices

ServiceReferenceBasePrivate::~ServiceReferenceBasePrivate() = default;

std::shared_ptr<ServiceRegistrationLocks>
ServiceRegistrationLocks
ServiceReferenceBasePrivate::LockServiceRegistration() const
{
return std::make_shared<ServiceRegistrationLocks>(registration.lock(), coreInfo);
return ServiceRegistrationLocks(registration.lock(), coreInfo);
}

InterfaceMapConstPtr
Expand Down
2 changes: 1 addition & 1 deletion framework/src/service/ServiceReferenceBasePrivate.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ namespace cppmicroservices

~ServiceReferenceBasePrivate();

std::shared_ptr<ServiceRegistrationLocks> LockServiceRegistration() const;
ServiceRegistrationLocks LockServiceRegistration() const;

/**
* Get the service object.
Expand Down
13 changes: 5 additions & 8 deletions framework/src/service/ServiceRegistrationBase.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -330,10 +330,10 @@ namespace cppmicroservices
}
}

std::shared_ptr<ServiceRegistrationLocks>
ServiceRegistrationLocks
ServiceRegistrationBase::LockServiceRegistration() const
{
return std::make_shared<ServiceRegistrationLocks>(d, d->coreInfo);
return ServiceRegistrationLocks(d, d->coreInfo);
}

bool
Expand All @@ -353,12 +353,9 @@ namespace cppmicroservices
return true;
}

ServiceReferenceBase sr1;
ServiceReferenceBase sr2;
{
LockServiceRegistration(), sr1 = d->reference;
o.LockServiceRegistration(), sr2 = o.d->reference;
}
ServiceReferenceBase sr1 = (LockServiceRegistration(), d->reference);
ServiceReferenceBase sr2 = (o.LockServiceRegistration(), o.d->reference);

return sr1 < sr2;
tcormackMW marked this conversation as resolved.
Show resolved Hide resolved
}

Expand Down
13 changes: 5 additions & 8 deletions framework/src/util/ServiceRegistrationLocks.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,14 +24,11 @@
namespace cppmicroservices
{

ServiceRegistrationLocks::ServiceRegistrationLocks(std::shared_ptr<ServiceRegistrationBasePrivate> reg,
std::shared_ptr<ServiceRegistrationCoreInfo> coreInfo)
{
if (reg != nullptr)
{
regL = reg->Lock();
}
coreInfoL = coreInfo->Lock();
ServiceRegistrationLocks::ServiceRegistrationLocks(const std::shared_ptr<ServiceRegistrationBasePrivate>& reg,
const std::shared_ptr<ServiceRegistrationCoreInfo>& coreInfo)
: regL(reg != nullptr ? reg->Lock() : cppmicroservices::detail::MutexLockingStrategy<>::UniqueLock())
, coreInfoL(coreInfo->Lock())
{
}
} // namespace cppmicroservices

Expand Down
12 changes: 9 additions & 3 deletions framework/src/util/ServiceRegistrationLocks.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,18 @@ namespace cppmicroservices
class ServiceRegistrationLocks final
{
public:
ServiceRegistrationLocks(std::shared_ptr<ServiceRegistrationBasePrivate> reg,
std::shared_ptr<ServiceRegistrationCoreInfo> coreInfo);
ServiceRegistrationLocks(const std::shared_ptr<ServiceRegistrationBasePrivate>& reg,
const std::shared_ptr<ServiceRegistrationCoreInfo>& coreInfo);

// Delete all copy and move to enforce that it is only ever constructed into one object -- avoids deadlocks
ServiceRegistrationLocks(ServiceRegistrationLocks const& lockObj) = delete;
ServiceRegistrationLocks(ServiceRegistrationLocks&& lockObj) = default;
ServiceRegistrationLocks& operator=(ServiceRegistrationLocks const& lockObj) = delete;
ServiceRegistrationLocks& operator=(ServiceRegistrationLocks&& lockObj) = delete;

private:
cppmicroservices::detail::MutexLockingStrategy<>::UniqueLock coreInfoL;
cppmicroservices::detail::MutexLockingStrategy<>::UniqueLock regL;
cppmicroservices::detail::MutexLockingStrategy<>::UniqueLock coreInfoL;
};
} // namespace cppmicroservices

Expand Down
Loading