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

feat: Implement Copy-On-Write (CoW) behavior for Logger struct #297

Merged
merged 21 commits into from
May 30, 2024
Merged

feat: Implement Copy-On-Write (CoW) behavior for Logger struct #297

merged 21 commits into from
May 30, 2024

Conversation

ayushi2103
Copy link
Contributor

@ayushi2103 ayushi2103 commented May 9, 2024 •
edited
Loading

Added CoW implementation for Logger struct to optimize performance and minimize memory overhead.

Motivation:

LogHandler is an existential type and has a String requiring two words so copying a Logger instance duplicates 7 words and executes two ARC operations. By implementing CoW with a boxed LogHandler, we will duplicate 1 word and one ARC operation, enhancing efficiency particularly when Logger instances are frequently passed around.

Modifications:

Added a private property _handler of type Box<LogHandler> to hold the LogHandler and updated the handler property to be computed, allowing transparent access to the LogHandler.

Result:

When the Logger instances will be passed around, the change will reduce memory overhead and potentially improve performance.

@ayushi2103
Copy link
Contributor Author

In which file should I add the unit test?

FranzBusch
FranzBusch reviewed May 9, 2024
View reviewed changes
Sources/Logging/Logging.swift Outdated Show resolved Hide resolved
Sources/Logging/Logging.swift Outdated Show resolved Hide resolved
Sources/Logging/Logging.swift Outdated Show resolved Hide resolved
@ayushi2103 ayushi2103 mentioned this pull request May 9, 2024
Closed
@FranzBusch
Copy link
Member

@swift-server-bot test this please

@ayushi2103 ayushi2103 marked this pull request as ready for review May 10, 2024 00:18
FranzBusch
FranzBusch reviewed May 14, 2024
View reviewed changes
Sources/Logging/Logging.swift Outdated Show resolved Hide resolved
Sources/Logging/Logging.swift Show resolved Hide resolved
@FranzBusch
Copy link
Member

@ayushi2103 Can you update this PR since we just landed #299 which drops support for old Swift versions. This should mean you no longer have to add the conditional conformance.

ayushi2103 and others added 2 commits May 17, 2024 08:40
@ayushi2103 @weissi
Update Sources/Logging/Logging.swift
7bd8869 
Co-authored-by: Johannes Weiss <johannesweiss@apple.com>
@ayushi2103 @weissi
Update Sources/Logging/Logging.swift
9e5e257 
Co-authored-by: Johannes Weiss <johannesweiss@apple.com>
FranzBusch
FranzBusch reviewed May 17, 2024
View reviewed changes
Sources/Logging/Logging.swift Outdated Show resolved Hide resolved
Sources/Logging/Logging.swift Show resolved Hide resolved
Sources/Logging/Logging.swift Show resolved Hide resolved
Sources/Logging/Logging.swift Outdated Show resolved Hide resolved
Sources/Logging/Logging.swift Outdated Show resolved Hide resolved
Sources/Logging/Logging.swift Outdated Show resolved Hide resolved
ayushi2103 and others added 4 commits May 17, 2024 18:51
FranzBusch
FranzBusch approved these changes May 22, 2024
View reviewed changes
Copy link
Member

@FranzBusch FranzBusch left a comment

Choose a reason for hiding this comment

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

This looks good to me! @weissi you want to take a last look as well before we merge?

weissi
weissi reviewed May 29, 2024
View reviewed changes
Copy link
Member

@weissi weissi left a comment

Choose a reason for hiding this comment

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

Thank you, this is almost there. Two little suggestions

@ayushi2103 ayushi2103 requested a review from weissi May 30, 2024 09:51
weissi
weissi approved these changes May 30, 2024
View reviewed changes
Copy link
Member

@weissi weissi left a comment

Choose a reason for hiding this comment

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

Amazing! Thank you so much, this will be quite the perf boost for certain things!!

@weissi weissi merged commit 2351723 into apple:main May 30, 2024
6 checks passed
@ayushi2103 ayushi2103 deleted the Logger-Copy-on-Write branch May 30, 2024 10:20
cgrindel-self-hosted-renovate bot added a commit to cgrindel/rules_swift_package_manager that referenced this pull request Jun 27, 2024
@cgrindel-self-hosted-renovate
chore(deps): update dependency apple/swift-log to from: "1.6.1" (#1122)
9fd4d54 
This PR contains the following updates:

| Package | Update | Change |
|---|---|---|
| [apple/swift-log](https://github.com/apple/swift-log) | minor |
`from: "1.5.4"` -> `from: "1.6.1"` |

---

### Release Notes

<details>
<summary>apple/swift-log (apple/swift-log)</summary>

### [`v1.6.1`](https://github.com/apple/swift-log/releases/tag/1.6.1):
Swift Log 1.6.1

[Compare
Source](https://github.com/apple/swift-log/compare/1.6.0...1.6.1)

##### SemVer Patch

- Disable existential any build setting
([#&#8203;312](https://github.com/apple/swift-log/issues/312))

### [`v1.6.0`](https://github.com/apple/swift-log/releases/tag/1.6.0)

[Compare
Source](https://github.com/apple/swift-log/compare/1.5.4...1.6.0)

#### SemVer Minor

- Add Sendability annotations in
[apple/swift-log#308
- Fix deprecation warnings around default log implementations on
handlers in
[apple/swift-log#310
- Drop Swift versions earlier than 5.8 in
[apple/swift-log#299
- Implement Copy-On-Write (CoW) behavior for Logger struct by
[@&#8203;ayushi2103](https://github.com/ayushi2103) in
[apple/swift-log#297

##### SemVer Patch

- Replace standardOutput to standardError by
[@&#8203;ayushi2103](https://github.com/ayushi2103) in
[apple/swift-log#295
- Use Set to spot duplicated log handler warnings in
[apple/swift-log#306
- Make protocol usage obvious using any and some keywords in
[apple/swift-log#307
- Remove documentation for non-existent arguments by
[@&#8203;b1ackturtle](https://github.com/b1ackturtle) in
[apple/swift-log#309
- Remove Docc plugin which is no longer required in
[apple/swift-log#311

##### Other Changes

- Remove archived repository in
[apple/swift-log#292
- Add CI for Swift 5.10 in
[apple/swift-log#287
- Added swift-log-ecs to README.md by
[@&#8203;rwbutler](https://github.com/rwbutler) in
[apple/swift-log#298
- Update README.md add shipbook as backend by
[@&#8203;elishas](https://github.com/elishas) in
[apple/swift-log#304

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR is behind base branch, or you tick the
rebase/retry checkbox.

👻 **Immortal**: This PR will be recreated if closed unmerged. Get
[config help](https://github.com/renovatebot/renovate/discussions) if
that's undesired.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Renovate
Bot](https://github.com/renovatebot/renovate).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNi4xMDkuNCIsInVwZGF0ZWRJblZlciI6IjM2LjEwOS40IiwidGFyZ2V0QnJhbmNoIjoibWFpbiJ9-->

Co-authored-by: Self-hosted Renovate Bot <361546+cgrindel-self-hosted-renovate[bot]@users.noreply.github.enterprise.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@weissi weissi weissi approved these changes

@FranzBusch FranzBusch FranzBusch approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ayushi2103 @FranzBusch @weissi