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

use DISALLOW_COPY_AND_MOVE instead. #4411

Closed
ywqzzy opened this issue Mar 24, 2022 · 5 comments
Closed

use DISALLOW_COPY_AND_MOVE instead. #4411

ywqzzy opened this issue Mar 24, 2022 · 5 comments
Labels
good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. type/code-quality-improvement PR that can improve the code quality type/enhancement The issue or PR belongs to an enhancement.

Comments

@ywqzzy
Copy link
Contributor

ywqzzy commented Mar 24, 2022 •
edited
Loading

Enhancement

reference:
https://github.com/oceanbase/oceanbase/blob/master/src/observer/ob_cache_size_calculator.h

In CPUAffinityManager.h and many other classes, they disabled copy and move constructor, maybe we can simpilfy the code like:

#define DISALLOW_COPY_AND_MOVE(className) \
    DISALLOW_COPY(className);             \
    DISALLOW_MOVE(className)

#define DISALLOW_COPY(className)      \
    className(const className&) = delete; \
    className& operator=(const className&) = delete

#define DISALLOW_MOVE(className) \
    className(className&&) = delete; \
    className& operator=(className&&) = delete
@ywqzzy ywqzzy added the type/enhancement The issue or PR belongs to an enhancement. label Mar 24, 2022
@ywqzzy ywqzzy changed the title Disallow copy and move. use DISALLOW_COPY_AND_MOVE instead. Mar 24, 2022
@ywqzzy ywqzzy added type/code-quality-improvement PR that can improve the code quality good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. labels Mar 24, 2022
@ZHANGWENTAI
Copy link
Contributor

may I have a try?

@ywqzzy
Copy link
Contributor Author

ywqzzy commented Apr 1, 2022

may I have a try?

You can try in one class at first.

@ZHANGWENTAI ZHANGWENTAI mentioned this issue Apr 2, 2022
Closed
12 tasks
@ZHANGWENTAI
Copy link
Contributor

Hi, I add the DISALLOW_COPY_AND_MOVE(className) macro in CPUAffinityManager.h. I wonder where should I write this macro so that all other class neet it can use it.
can I ask you to review my code?

@ZHANGWENTAI ZHANGWENTAI mentioned this issue Apr 2, 2022
Merged
12 tasks
@ZHANGWENTAI
Copy link
Contributor

ZHANGWENTAI commented Apr 12, 2022 •
edited
Loading

there are some classes I intend to refactor:
disallow copy:

dbms/src/Common/ActionBlocker.h:
63: DISALLOW_COPY(LockHolder);

dbms/src/Common/CompactArray.h:
109: DISALLOW_COPY(Reader);

dbms/src/Common/COWPtr.h:
125: DISALLOW_COPY(mutable_ptr);

dbms/src/Common/CPUAffinityManager.h:
130: DISALLOW_COPY_AND_MOVE(CPUAffinityManager);

dbms/src/Common/DNSCache.h:
31: DISALLOW_COPY(DNSCache);

dbms/src/Common/ExecutableTask.h:
29: DISALLOW_COPY_AND_MOVE(IExecutableTask);

dbms/src/Common/RWLock.cpp:
62: DISALLOW_COPY(LockHolderImpl);

dbms/src/Common/TiFlashMetrics.h:
345: DISALLOW_COPY_AND_MOVE(TiFlashMetrics);

dbms/src/Common/HashTable/FixedHashTable.h:
262: DISALLOW_COPY(Reader);

dbms/src/Common/HashTable/HashTable.h:
775: DISALLOW_COPY(Reader);

dbms/src/Common/HashTable/SmallTable.h:
98: DISALLOW_COPY(Reader);

dbms/src/Common/tests/gtest_mpmc_queue.cpp:
470: DISALLOW_COPY(ThrowInjectable);

dbms/src/DataStreams/SummingSortedBlockInputStream.h:
125: DISALLOW_COPY(AggregateDescription);

dbms/src/Debug/MockSSTReader.h:
40: DISALLOW_COPY(Data);

dbms/src/Encryption/createWriteBufferFromFileBaseByFileProvider.h:
100: DISALLOW_COPY(WriteBufferByFileProviderBuilder);

dbms/src/Encryption/RateLimiter.h:
266: DISALLOW_COPY_AND_MOVE(IORateLimiter);

dbms/src/Interpreters/LogicalExpressionsOptimizer.h:
50: DISALLOW_COPY(LogicalExpressionsOptimizer);

dbms/src/IO/DoubleConverter.h:
56: DISALLOW_COPY(DoubleConverter);

dbms/src/IO/ReadBufferAIO.h:
52: DISALLOW_COPY(ReadBufferAIO);

dbms/src/IO/WriteBufferAIO.h:
52: DISALLOW_COPY(WriteBufferAIO);

dbms/src/Storages/DeltaMerge/Segment.h:
109: DISALLOW_COPY_AND_MOVE(Segment);

dbms/src/Storages/DeltaMerge/ColumnFile/ColumnFile.h:
134: DISALLOW_COPY(ColumnFileReader);

dbms/src/Storages/DeltaMerge/tools/workload/TableGenerator.cpp:
48: DISALLOW_COPY_AND_MOVE(TablePkType);

dbms/src/Storages/Page/stress/PSWorkload.h:
130: DISALLOW_COPY_AND_MOVE(StressWorkloadManger);

dbms/src/Storages/Page/V1/PageEntries.h:
202: DISALLOW_COPY(PageEntriesMixin);

dbms/src/Storages/Page/V1/mvcc/VersionSet.h:
223: DISALLOW_COPY(Snapshot);
260: DISALLOW_COPY(VersionSet);

dbms/src/Storages/Page/V1/VersionSet/PageEntriesEdit.h:
86: DISALLOW_COPY(PageEntriesEdit);

dbms/src/Storages/Page/V2/PageEntries.h:
278: DISALLOW_COPY(PageEntriesMixin);

dbms/src/Storages/Page/V2/VersionSet/PageEntriesEdit.h:
94: DISALLOW_COPY(PageEntriesEdit);

dbms/src/Storages/Page/V3/PageDirectory.h:
363: DISALLOW_COPY_AND_MOVE(PageDirectory);

dbms/src/Storages/Page/V3/PageEntriesEdit.h:
267: DISALLOW_COPY(PageEntriesEdit);

dbms/src/Storages/Page/V3/LogFile/LogReader.h:
55: DISALLOW_COPY(LogReader);

dbms/src/Storages/Page/V3/LogFile/LogWriter.h:
84: DISALLOW_COPY(LogWriter);

dbms/src/Storages/Page/V3/WAL/WALReader.h:
85: DISALLOW_COPY(WALStoreReader);

dbms/src/Storages/Transaction/DecodingStorageSchemaSnapshot.h:
143: DISALLOW_COPY(DecodingStorageSchemaSnapshot);

dbms/src/Storages/Transaction/FileEncryption.h:
61: DISALLOW_COPY(FileEncryptionInfo);

dbms/src/Storages/Transaction/ProxyFFI.cpp:
257: DISALLOW_COPY(CppStrVec);

dbms/src/Storages/Transaction/ProxyFFI.h:
71: DISALLOW_COPY(RawRustPtrWrap);

dbms/src/Storages/Transaction/ProxyFFICommon.h:
29: DISALLOW_COPY(RawCppString);

dbms/src/Storages/Transaction/ReadIndexWorker.h:
209: DISALLOW_COPY(ReadIndexElement);

dbms/src/Storages/Transaction/RegionTable.h:
221: DISALLOW_COPY(RegionPreDecodeBlockData);

dbms/src/Storages/Transaction/SSTReader.h:
30: DISALLOW_COPY_AND_MOVE(SSTReader);

dbms/src/Storages/Transaction/TableRowIDMinMax.h:
32: DISALLOW_COPY_AND_MOVE(TableRowIDMinMax);

dbms/src/Storages/Transaction/TiKVKeyValue.h:
50: DISALLOW_COPY(StringObject);

disallow move:

dbms/src/Common/CPUAffinityManager.h:
130: DISALLOW_COPY_AND_MOVE(CPUAffinityManager);

dbms/src/Common/ExecutableTask.h:
29: DISALLOW_COPY_AND_MOVE(IExecutableTask);

dbms/src/Common/TiFlashMetrics.h:
345: DISALLOW_COPY_AND_MOVE(TiFlashMetrics);

dbms/src/Encryption/RateLimiter.h:
266: DISALLOW_COPY_AND_MOVE(IORateLimiter);

dbms/src/Storages/DeltaMerge/Segment.h:
109: DISALLOW_COPY_AND_MOVE(Segment);

dbms/src/Storages/DeltaMerge/tools/workload/TableGenerator.cpp:
48: DISALLOW_COPY_AND_MOVE(TablePkType);

dbms/src/Storages/Page/stress/PSWorkload.h:
130: DISALLOW_COPY_AND_MOVE(StressWorkloadManger);

dbms/src/Storages/Page/V3/PageDirectory.h:
363: DISALLOW_COPY_AND_MOVE(PageDirectory);

dbms/src/Storages/Transaction/SSTReader.h:
30: DISALLOW_COPY_AND_MOVE(SSTReader);

dbms/src/Storages/Transaction/TableRowIDMinMax.h:
32: DISALLOW_COPY_AND_MOVE(TableRowIDMinMax);

dbms/src/Flash/Coprocessor/DAGStorageInterpreter.h:
59: DISALLOW_MOVE(DAGStorageInterpreter);

dbms/src/IO/WriteBufferFromString.h:
64: DISALLOW_MOVE(WriteBufferFromOwnString);

any suggestions?

ti-chi-bot pushed a commit that referenced this issue May 7, 2022
@ZHANGWENTAI
Disallow copy and move (#4578)
7ffb943 
ref #4411
@ywqzzy ywqzzy closed this as completed May 9, 2022
@breezewish
Copy link
Member

I see that there are also boost::noncopyable in the code base. Maybe we can unify them?

@Lloyd-Pottiger Lloyd-Pottiger mentioned this issue May 19, 2022
Closed
12 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. type/code-quality-improvement PR that can improve the code quality type/enhancement The issue or PR belongs to an enhancement.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

add DISALLOW_COPY_AND_ASSIGN macro for CPUAffinityManager ZHANGWENTAI/tiflash
3 participants
@breezewish @ywqzzy @ZHANGWENTAI