You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@dubbo.apache.org by GitBox <gi...@apache.org> on 2021/10/30 04:28:14 UTC
[GitHub] [dubbo-getty] Lvnszn opened a new pull request #82: WIP: Opt: split large packet to n*16KB packet
Lvnszn opened a new pull request #82:
URL: https://github.com/apache/dubbo-getty/pull/82
<!-- Thanks for sending a pull request!
-->
**What this PR does**:
split large packet to 16KB packet and send one by one for reduce risk when send large packet.
**Which issue(s) this PR fixes**:
<!--
*Automatically closes linked issue when PR is merged.
Usage: `Fixes #<issue number>`, or `Fixes (paste link of issue)`.
_If PR is about `failing-tests or flakes`, please post the related issues/tests in a comment and do not use `Fixes`_*
-->
Fixes #
**Special notes for your reviewer**:
**Does this PR introduce a user-facing change?**:
<!--
If no, just write "NONE" in the release-note block below.
If yes, a release note is required:
Enter your extended release note in the block below. If the PR requires additional action from users switching to the new release, include the string "action required".
-->
```release-note
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo-getty] wenxuwan commented on a change in pull request #82: Opt: split large packet to n*16KB packet
Posted by GitBox <gi...@apache.org>.
wenxuwan commented on a change in pull request #82:
URL: https://github.com/apache/dubbo-getty/pull/82#discussion_r749883543
##########
File path: session.go
##########
@@ -418,11 +422,28 @@ func (s *session) WriteBytes(pkg []byte) (int, error) {
return 0, ErrSessionClosed
}
- lg, err := s.Connection.send(pkg)
+ leftPackageSize, totalSize, writeSize := len(pkg), len(pkg), 0
+ s.wlock.Lock()
Review comment:
In my opinion, the wlock used for protect big package send, so may be u can change the wlock to rwMutex, and only when package size bigger than 16kb, try to get Lock, other times and other places you can use Rlock.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo-getty] codecov-commenter edited a comment on pull request #82: Opt: split large packet to n*16KB packet
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #82:
URL: https://github.com/apache/dubbo-getty/pull/82#issuecomment-955414110
# [Codecov](https://codecov.io/gh/apache/dubbo-getty/pull/82?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#82](https://codecov.io/gh/apache/dubbo-getty/pull/82?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (a3f9370) into [master](https://codecov.io/gh/apache/dubbo-getty/commit/a2461f81a132df705db157b591e00d8535ff6436?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (a2461f8) will **increase** coverage by `0.27%`.
> The diff coverage is `81.81%`.
[![Impacted file tree graph](https://codecov.io/gh/apache/dubbo-getty/pull/82/graphs/tree.svg?width=650&height=150&src=pr&token=WDmUsbxiLS&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/dubbo-getty/pull/82?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #82 +/- ##
==========================================
+ Coverage 68.87% 69.14% +0.27%
==========================================
Files 8 8
Lines 1420 1439 +19
==========================================
+ Hits 978 995 +17
- Misses 347 348 +1
- Partials 95 96 +1
```
| [Impacted Files](https://codecov.io/gh/apache/dubbo-getty/pull/82?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [session.go](https://codecov.io/gh/apache/dubbo-getty/pull/82/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Vzc2lvbi5nbw==) | `69.83% <81.81%> (+0.38%)` | :arrow_up: |
| [connection.go](https://codecov.io/gh/apache/dubbo-getty/pull/82/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29ubmVjdGlvbi5nbw==) | `80.39% <0.00%> (+0.65%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/dubbo-getty/pull/82?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/dubbo-getty/pull/82?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [a2461f8...a3f9370](https://codecov.io/gh/apache/dubbo-getty/pull/82?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo-getty] codecov-commenter edited a comment on pull request #82: Opt: split large packet to n*16KB packet
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #82:
URL: https://github.com/apache/dubbo-getty/pull/82#issuecomment-955414110
# [Codecov](https://codecov.io/gh/apache/dubbo-getty/pull/82?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#82](https://codecov.io/gh/apache/dubbo-getty/pull/82?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (a048da4) into [master](https://codecov.io/gh/apache/dubbo-getty/commit/a2461f81a132df705db157b591e00d8535ff6436?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (a2461f8) will **increase** coverage by `1.09%`.
> The diff coverage is `77.77%`.
[![Impacted file tree graph](https://codecov.io/gh/apache/dubbo-getty/pull/82/graphs/tree.svg?width=650&height=150&src=pr&token=WDmUsbxiLS&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/dubbo-getty/pull/82?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #82 +/- ##
==========================================
+ Coverage 68.87% 69.96% +1.09%
==========================================
Files 8 8
Lines 1420 1435 +15
==========================================
+ Hits 978 1004 +26
+ Misses 347 335 -12
- Partials 95 96 +1
```
| [Impacted Files](https://codecov.io/gh/apache/dubbo-getty/pull/82?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [session.go](https://codecov.io/gh/apache/dubbo-getty/pull/82/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Vzc2lvbi5nbw==) | `71.70% <77.77%> (+2.25%)` | :arrow_up: |
| [client.go](https://codecov.io/gh/apache/dubbo-getty/pull/82/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y2xpZW50Lmdv) | `73.46% <0.00%> (+1.92%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/dubbo-getty/pull/82?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/dubbo-getty/pull/82?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [a2461f8...a048da4](https://codecov.io/gh/apache/dubbo-getty/pull/82?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo-getty] AlexStocks commented on pull request #82: WIP: Opt: split large packet to n*16KB packet
Posted by GitBox <gi...@apache.org>.
AlexStocks commented on pull request #82:
URL: https://github.com/apache/dubbo-getty/pull/82#issuecomment-958660946
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo-getty] wongoo commented on a change in pull request #82: Opt: split large packet to n*16KB packet
Posted by GitBox <gi...@apache.org>.
wongoo commented on a change in pull request #82:
URL: https://github.com/apache/dubbo-getty/pull/82#discussion_r749254638
##########
File path: session.go
##########
@@ -418,11 +422,23 @@ func (s *session) WriteBytes(pkg []byte) (int, error) {
return 0, ErrSessionClosed
}
- lg, err := s.Connection.send(pkg)
+ s.wlock.Lock()
+ defer s.wlock.Unlock()
+ totalSize, writeSize := len(pkg), 0
+ for totalSize >= maxPacketLen {
+ _, err := s.Connection.send(pkg[writeSize:(writeSize + maxPacketLen)])
+ if err != nil {
+ return writeSize, perrors.Wrapf(err, "s.Connection.Write(pkg len:%d)", len(pkg))
+ }
+ totalSize -= maxPacketLen
+ writeSize += maxPacketLen
+ }
+ lg, err := s.Connection.send(pkg[writeSize:])
Review comment:
@Lvnszn yes, should return if totalSize already decreased to 0
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo-getty] Lvnszn commented on a change in pull request #82: Opt: split large packet to n*16KB packet
Posted by GitBox <gi...@apache.org>.
Lvnszn commented on a change in pull request #82:
URL: https://github.com/apache/dubbo-getty/pull/82#discussion_r749169202
##########
File path: session.go
##########
@@ -418,11 +422,23 @@ func (s *session) WriteBytes(pkg []byte) (int, error) {
return 0, ErrSessionClosed
}
- lg, err := s.Connection.send(pkg)
+ s.wlock.Lock()
+ defer s.wlock.Unlock()
+ totalSize, writeSize := len(pkg), 0
+ for totalSize >= maxPacketLen {
+ _, err := s.Connection.send(pkg[writeSize:(writeSize + maxPacketLen)])
+ if err != nil {
+ return writeSize, perrors.Wrapf(err, "s.Connection.Write(pkg len:%d)", len(pkg))
+ }
+ totalSize -= maxPacketLen
+ writeSize += maxPacketLen
+ }
+ lg, err := s.Connection.send(pkg[writeSize:])
Review comment:
IMO, the packet needn't to be send. Addressed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo-getty] codecov-commenter edited a comment on pull request #82: Opt: split large packet to n*16KB packet
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #82:
URL: https://github.com/apache/dubbo-getty/pull/82#issuecomment-955414110
# [Codecov](https://codecov.io/gh/apache/dubbo-getty/pull/82?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#82](https://codecov.io/gh/apache/dubbo-getty/pull/82?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (03338b9) into [master](https://codecov.io/gh/apache/dubbo-getty/commit/a2461f81a132df705db157b591e00d8535ff6436?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (a2461f8) will **increase** coverage by `1.09%`.
> The diff coverage is `77.77%`.
[![Impacted file tree graph](https://codecov.io/gh/apache/dubbo-getty/pull/82/graphs/tree.svg?width=650&height=150&src=pr&token=WDmUsbxiLS&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/dubbo-getty/pull/82?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #82 +/- ##
==========================================
+ Coverage 68.87% 69.96% +1.09%
==========================================
Files 8 8
Lines 1420 1435 +15
==========================================
+ Hits 978 1004 +26
+ Misses 347 335 -12
- Partials 95 96 +1
```
| [Impacted Files](https://codecov.io/gh/apache/dubbo-getty/pull/82?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [session.go](https://codecov.io/gh/apache/dubbo-getty/pull/82/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Vzc2lvbi5nbw==) | `71.70% <77.77%> (+2.25%)` | :arrow_up: |
| [client.go](https://codecov.io/gh/apache/dubbo-getty/pull/82/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y2xpZW50Lmdv) | `73.46% <0.00%> (+1.92%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/dubbo-getty/pull/82?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/dubbo-getty/pull/82?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [a2461f8...03338b9](https://codecov.io/gh/apache/dubbo-getty/pull/82?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo-getty] AlexStocks commented on a change in pull request #82: WIP: Opt: split large packet to n*16KB packet
Posted by GitBox <gi...@apache.org>.
AlexStocks commented on a change in pull request #82:
URL: https://github.com/apache/dubbo-getty/pull/82#discussion_r739624806
##########
File path: session.go
##########
@@ -418,11 +419,23 @@ func (s *session) WriteBytes(pkg []byte) (int, error) {
return 0, ErrSessionClosed
}
- lg, err := s.Connection.send(pkg)
+ totalSize := len(pkg)
Review comment:
totalSize := len(pkg)
pkgArray := make([][]byte, 0, totalSize / maxPacketLen + 1)
writeSize := 0
for totalSize >= maxPacketLen {
pkgArray = append(pkgArray, pkg[writeSize:(writeSize + maxPacketLen)])
totalSize -= maxPacketLen
writeSize += maxPacketLen
}
pkgArray = append(pkgArray, pkg[writeSize:])
lg, err := s.Connection.send(pkgArray)
if err != nil {
return writeSize, perrors.Wrapf(err, "s.Connection.Write(pkg len:%d)", len(pkg))
}
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo-getty] wenxuwan commented on a change in pull request #82: Opt: split large packet to n*16KB packet
Posted by GitBox <gi...@apache.org>.
wenxuwan commented on a change in pull request #82:
URL: https://github.com/apache/dubbo-getty/pull/82#discussion_r749849504
##########
File path: session.go
##########
@@ -418,11 +422,27 @@ func (s *session) WriteBytes(pkg []byte) (int, error) {
return 0, ErrSessionClosed
}
- lg, err := s.Connection.send(pkg)
+ totalSize := len(pkg)
+ if totalSize == 0 {
+ return 0, nil
+ }
+ writeSize := 0
+ s.wlock.Lock()
+ defer s.wlock.Unlock()
+ for totalSize >= maxPacketLen {
Review comment:
totalSize change to leftPackageSize or packageSize should be more readable?
##########
File path: session.go
##########
@@ -418,11 +422,27 @@ func (s *session) WriteBytes(pkg []byte) (int, error) {
return 0, ErrSessionClosed
}
- lg, err := s.Connection.send(pkg)
+ totalSize := len(pkg)
+ if totalSize == 0 {
+ return 0, nil
+ }
+ writeSize := 0
+ s.wlock.Lock()
+ defer s.wlock.Unlock()
+ for totalSize >= maxPacketLen {
+ _, err := s.Connection.send(pkg[writeSize:(writeSize + maxPacketLen)])
+ if err != nil {
+ return writeSize, perrors.Wrapf(err, "s.Connection.Write(pkg len:%d)", len(pkg))
+ }
+ totalSize -= maxPacketLen
+ writeSize += maxPacketLen
+ }
+ lg, err := s.Connection.send(pkg[writeSize:])
if err != nil {
- return 0, perrors.Wrapf(err, "s.Connection.Write(pkg len:%d)", len(pkg))
+ return writeSize, perrors.Wrapf(err, "s.Connection.Write(pkg len:%d)", len(pkg))
}
- return lg, nil
+
+ return writeSize + lg, nil
Review comment:
here can return totalsize directly ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo-getty] AlexStocks merged pull request #82: Opt: split large packet to n*16KB packet
Posted by GitBox <gi...@apache.org>.
AlexStocks merged pull request #82:
URL: https://github.com/apache/dubbo-getty/pull/82
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo-getty] codecov-commenter edited a comment on pull request #82: Opt: split large packet to n*16KB packet
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #82:
URL: https://github.com/apache/dubbo-getty/pull/82#issuecomment-955414110
# [Codecov](https://codecov.io/gh/apache/dubbo-getty/pull/82?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#82](https://codecov.io/gh/apache/dubbo-getty/pull/82?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (03338b9) into [master](https://codecov.io/gh/apache/dubbo-getty/commit/a2461f81a132df705db157b591e00d8535ff6436?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (a2461f8) will **increase** coverage by `1.23%`.
> The diff coverage is `77.77%`.
[![Impacted file tree graph](https://codecov.io/gh/apache/dubbo-getty/pull/82/graphs/tree.svg?width=650&height=150&src=pr&token=WDmUsbxiLS&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/dubbo-getty/pull/82?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #82 +/- ##
==========================================
+ Coverage 68.87% 70.10% +1.23%
==========================================
Files 8 8
Lines 1420 1435 +15
==========================================
+ Hits 978 1006 +28
+ Misses 347 334 -13
Partials 95 95
```
| [Impacted Files](https://codecov.io/gh/apache/dubbo-getty/pull/82?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [session.go](https://codecov.io/gh/apache/dubbo-getty/pull/82/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Vzc2lvbi5nbw==) | `71.70% <77.77%> (+2.25%)` | :arrow_up: |
| [connection.go](https://codecov.io/gh/apache/dubbo-getty/pull/82/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29ubmVjdGlvbi5nbw==) | `80.39% <0.00%> (+0.65%)` | :arrow_up: |
| [client.go](https://codecov.io/gh/apache/dubbo-getty/pull/82/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y2xpZW50Lmdv) | `73.46% <0.00%> (+1.92%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/dubbo-getty/pull/82?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/dubbo-getty/pull/82?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [a2461f8...03338b9](https://codecov.io/gh/apache/dubbo-getty/pull/82?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo-getty] codecov-commenter edited a comment on pull request #82: Opt: split large packet to n*16KB packet
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #82:
URL: https://github.com/apache/dubbo-getty/pull/82#issuecomment-955414110
# [Codecov](https://codecov.io/gh/apache/dubbo-getty/pull/82?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#82](https://codecov.io/gh/apache/dubbo-getty/pull/82?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (c3d9e00) into [master](https://codecov.io/gh/apache/dubbo-getty/commit/a2461f81a132df705db157b591e00d8535ff6436?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (a2461f8) will **increase** coverage by `1.25%`.
> The diff coverage is `78.94%`.
[![Impacted file tree graph](https://codecov.io/gh/apache/dubbo-getty/pull/82/graphs/tree.svg?width=650&height=150&src=pr&token=WDmUsbxiLS&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/dubbo-getty/pull/82?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #82 +/- ##
==========================================
+ Coverage 68.87% 70.12% +1.25%
==========================================
Files 8 8
Lines 1420 1436 +16
==========================================
+ Hits 978 1007 +29
+ Misses 347 334 -13
Partials 95 95
```
| [Impacted Files](https://codecov.io/gh/apache/dubbo-getty/pull/82?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [session.go](https://codecov.io/gh/apache/dubbo-getty/pull/82/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Vzc2lvbi5nbw==) | `71.76% <78.94%> (+2.31%)` | :arrow_up: |
| [connection.go](https://codecov.io/gh/apache/dubbo-getty/pull/82/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29ubmVjdGlvbi5nbw==) | `80.39% <0.00%> (+0.65%)` | :arrow_up: |
| [client.go](https://codecov.io/gh/apache/dubbo-getty/pull/82/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y2xpZW50Lmdv) | `73.46% <0.00%> (+1.92%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/dubbo-getty/pull/82?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/dubbo-getty/pull/82?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [a2461f8...c3d9e00](https://codecov.io/gh/apache/dubbo-getty/pull/82?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo-getty] codecov-commenter edited a comment on pull request #82: Opt: split large packet to n*16KB packet
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #82:
URL: https://github.com/apache/dubbo-getty/pull/82#issuecomment-955414110
# [Codecov](https://codecov.io/gh/apache/dubbo-getty/pull/82?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#82](https://codecov.io/gh/apache/dubbo-getty/pull/82?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (a048da4) into [master](https://codecov.io/gh/apache/dubbo-getty/commit/a2461f81a132df705db157b591e00d8535ff6436?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (a2461f8) will **increase** coverage by `1.23%`.
> The diff coverage is `77.77%`.
[![Impacted file tree graph](https://codecov.io/gh/apache/dubbo-getty/pull/82/graphs/tree.svg?width=650&height=150&src=pr&token=WDmUsbxiLS&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/dubbo-getty/pull/82?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #82 +/- ##
==========================================
+ Coverage 68.87% 70.10% +1.23%
==========================================
Files 8 8
Lines 1420 1435 +15
==========================================
+ Hits 978 1006 +28
+ Misses 347 334 -13
Partials 95 95
```
| [Impacted Files](https://codecov.io/gh/apache/dubbo-getty/pull/82?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [session.go](https://codecov.io/gh/apache/dubbo-getty/pull/82/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Vzc2lvbi5nbw==) | `71.70% <77.77%> (+2.25%)` | :arrow_up: |
| [connection.go](https://codecov.io/gh/apache/dubbo-getty/pull/82/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29ubmVjdGlvbi5nbw==) | `80.39% <0.00%> (+0.65%)` | :arrow_up: |
| [client.go](https://codecov.io/gh/apache/dubbo-getty/pull/82/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y2xpZW50Lmdv) | `73.46% <0.00%> (+1.92%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/dubbo-getty/pull/82?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/dubbo-getty/pull/82?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [a2461f8...a048da4](https://codecov.io/gh/apache/dubbo-getty/pull/82?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo-getty] AlexStocks commented on a change in pull request #82: Opt: split large packet to n*16KB packet
Posted by GitBox <gi...@apache.org>.
AlexStocks commented on a change in pull request #82:
URL: https://github.com/apache/dubbo-getty/pull/82#discussion_r749906529
##########
File path: session.go
##########
@@ -418,11 +422,28 @@ func (s *session) WriteBytes(pkg []byte) (int, error) {
return 0, ErrSessionClosed
}
- lg, err := s.Connection.send(pkg)
+ leftPackageSize, totalSize, writeSize := len(pkg), len(pkg), 0
+ s.wlock.Lock()
Review comment:
good idea
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo-getty] AlexStocks commented on pull request #82: WIP: Opt: split large packet to n*16KB packet
Posted by GitBox <gi...@apache.org>.
AlexStocks commented on pull request #82:
URL: https://github.com/apache/dubbo-getty/pull/82#issuecomment-958660946
pr 我看了,没多大毛病。关键还是发送大包时,要不要加锁的问题。
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo-getty] Lvnszn commented on a change in pull request #82: Opt: split large packet to n*16KB packet
Posted by GitBox <gi...@apache.org>.
Lvnszn commented on a change in pull request #82:
URL: https://github.com/apache/dubbo-getty/pull/82#discussion_r750332959
##########
File path: session.go
##########
@@ -418,11 +422,28 @@ func (s *session) WriteBytes(pkg []byte) (int, error) {
return 0, ErrSessionClosed
}
- lg, err := s.Connection.send(pkg)
+ leftPackageSize, totalSize, writeSize := len(pkg), len(pkg), 0
+ s.wlock.Lock()
Review comment:
address
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo-getty] wongoo commented on a change in pull request #82: Opt: split large packet to n*16KB packet
Posted by GitBox <gi...@apache.org>.
wongoo commented on a change in pull request #82:
URL: https://github.com/apache/dubbo-getty/pull/82#discussion_r748951212
##########
File path: session.go
##########
@@ -418,11 +422,23 @@ func (s *session) WriteBytes(pkg []byte) (int, error) {
return 0, ErrSessionClosed
}
- lg, err := s.Connection.send(pkg)
+ s.wlock.Lock()
+ defer s.wlock.Unlock()
+ totalSize, writeSize := len(pkg), 0
+ for totalSize >= maxPacketLen {
+ _, err := s.Connection.send(pkg[writeSize:(writeSize + maxPacketLen)])
+ if err != nil {
+ return writeSize, perrors.Wrapf(err, "s.Connection.Write(pkg len:%d)", len(pkg))
+ }
+ totalSize -= maxPacketLen
+ writeSize += maxPacketLen
+ }
+ lg, err := s.Connection.send(pkg[writeSize:])
Review comment:
does it need to send if the `totalSize = 0`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo-getty] Lvnszn commented on a change in pull request #82: Opt: split large packet to n*16KB packet
Posted by GitBox <gi...@apache.org>.
Lvnszn commented on a change in pull request #82:
URL: https://github.com/apache/dubbo-getty/pull/82#discussion_r749167270
##########
File path: session.go
##########
@@ -418,11 +419,23 @@ func (s *session) WriteBytes(pkg []byte) (int, error) {
return 0, ErrSessionClosed
}
- lg, err := s.Connection.send(pkg)
+ totalSize := len(pkg)
Review comment:
address
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo-getty] Lvnszn commented on a change in pull request #82: Opt: split large packet to n*16KB packet
Posted by GitBox <gi...@apache.org>.
Lvnszn commented on a change in pull request #82:
URL: https://github.com/apache/dubbo-getty/pull/82#discussion_r749169395
##########
File path: session.go
##########
@@ -129,6 +130,7 @@ type session struct {
// goroutines sync
grNum uatomic.Int32
lock sync.RWMutex
+ wlock sync.Mutex
Review comment:
addressed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo-getty] AlexStocks commented on pull request #82: WIP: Opt: split large packet to n*16KB packet
Posted by GitBox <gi...@apache.org>.
AlexStocks commented on pull request #82:
URL: https://github.com/apache/dubbo-getty/pull/82#issuecomment-958660946
pr 我看了,没多大毛病。关键还是发送大包时,要不要加锁的问题。
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo-getty] AlexStocks commented on a change in pull request #82: Opt: split large packet to n*16KB packet
Posted by GitBox <gi...@apache.org>.
AlexStocks commented on a change in pull request #82:
URL: https://github.com/apache/dubbo-getty/pull/82#discussion_r748700010
##########
File path: session.go
##########
@@ -129,6 +130,7 @@ type session struct {
// goroutines sync
grNum uatomic.Int32
lock sync.RWMutex
+ wlock sync.Mutex
Review comment:
u should use this wlock before every write action/funcs.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo-getty] codecov-commenter edited a comment on pull request #82: Opt: split large packet to n*16KB packet
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #82:
URL: https://github.com/apache/dubbo-getty/pull/82#issuecomment-955414110
# [Codecov](https://codecov.io/gh/apache/dubbo-getty/pull/82?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#82](https://codecov.io/gh/apache/dubbo-getty/pull/82?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (56c79d7) into [master](https://codecov.io/gh/apache/dubbo-getty/commit/a2461f81a132df705db157b591e00d8535ff6436?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (a2461f8) will **increase** coverage by `1.18%`.
> The diff coverage is `87.50%`.
[![Impacted file tree graph](https://codecov.io/gh/apache/dubbo-getty/pull/82/graphs/tree.svg?width=650&height=150&src=pr&token=WDmUsbxiLS&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/dubbo-getty/pull/82?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #82 +/- ##
==========================================
+ Coverage 68.87% 70.06% +1.18%
==========================================
Files 8 8
Lines 1420 1433 +13
==========================================
+ Hits 978 1004 +26
+ Misses 347 334 -13
Partials 95 95
```
| [Impacted Files](https://codecov.io/gh/apache/dubbo-getty/pull/82?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [session.go](https://codecov.io/gh/apache/dubbo-getty/pull/82/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Vzc2lvbi5nbw==) | `72.00% <87.50%> (+2.55%)` | :arrow_up: |
| [client.go](https://codecov.io/gh/apache/dubbo-getty/pull/82/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y2xpZW50Lmdv) | `73.46% <0.00%> (+1.92%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/dubbo-getty/pull/82?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/dubbo-getty/pull/82?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [a2461f8...56c79d7](https://codecov.io/gh/apache/dubbo-getty/pull/82?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo-getty] codecov-commenter commented on pull request #82: WIP: Opt: split large packet to n*16KB packet
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #82:
URL: https://github.com/apache/dubbo-getty/pull/82#issuecomment-955414110
# [Codecov](https://codecov.io/gh/apache/dubbo-getty/pull/82?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#82](https://codecov.io/gh/apache/dubbo-getty/pull/82?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (7ab8a1f) into [master](https://codecov.io/gh/apache/dubbo-getty/commit/a2461f81a132df705db157b591e00d8535ff6436?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (a2461f8) will **decrease** coverage by `0.01%`.
> The diff coverage is `71.42%`.
[![Impacted file tree graph](https://codecov.io/gh/apache/dubbo-getty/pull/82/graphs/tree.svg?width=650&height=150&src=pr&token=WDmUsbxiLS&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/dubbo-getty/pull/82?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #82 +/- ##
==========================================
- Coverage 68.87% 68.85% -0.02%
==========================================
Files 8 8
Lines 1420 1432 +12
==========================================
+ Hits 978 986 +8
- Misses 347 350 +3
- Partials 95 96 +1
```
| [Impacted Files](https://codecov.io/gh/apache/dubbo-getty/pull/82?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [session.go](https://codecov.io/gh/apache/dubbo-getty/pull/82/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Vzc2lvbi5nbw==) | `69.11% <55.55%> (-0.34%)` | :arrow_down: |
| [connection.go](https://codecov.io/gh/apache/dubbo-getty/pull/82/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29ubmVjdGlvbi5nbw==) | `80.00% <100.00%> (+0.26%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/dubbo-getty/pull/82?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/dubbo-getty/pull/82?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [a2461f8...7ab8a1f](https://codecov.io/gh/apache/dubbo-getty/pull/82?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo-getty] AlexStocks commented on a change in pull request #82: WIP: Opt: split large packet to n*16KB packet
Posted by GitBox <gi...@apache.org>.
AlexStocks commented on a change in pull request #82:
URL: https://github.com/apache/dubbo-getty/pull/82#discussion_r739674647
##########
File path: session.go
##########
@@ -418,10 +419,20 @@ func (s *session) WriteBytes(pkg []byte) (int, error) {
return 0, ErrSessionClosed
}
- lg, err := s.Connection.send(pkg)
+ totalSize := len(pkg)
+ pkgs := make([][]byte, 0, totalSize/maxPacketLen+1)
+ writeSize := 0
+ for totalSize >= maxPacketLen {
+ pkgs = append(pkgs, pkg[writeSize:(writeSize+maxPacketLen)])
+ totalSize -= maxPacketLen
+ writeSize += maxPacketLen
+ }
+ pkgs = append(pkgs, pkg[writeSize:])
+ lg, err := s.Connection.send(pkgs)
Review comment:
u need not to change the codes so quickly. pls test it firstly.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo-getty] wongoo commented on a change in pull request #82: Opt: split large packet to n*16KB packet
Posted by GitBox <gi...@apache.org>.
wongoo commented on a change in pull request #82:
URL: https://github.com/apache/dubbo-getty/pull/82#discussion_r748951212
##########
File path: session.go
##########
@@ -418,11 +422,23 @@ func (s *session) WriteBytes(pkg []byte) (int, error) {
return 0, ErrSessionClosed
}
- lg, err := s.Connection.send(pkg)
+ s.wlock.Lock()
+ defer s.wlock.Unlock()
+ totalSize, writeSize := len(pkg), 0
+ for totalSize >= maxPacketLen {
+ _, err := s.Connection.send(pkg[writeSize:(writeSize + maxPacketLen)])
+ if err != nil {
+ return writeSize, perrors.Wrapf(err, "s.Connection.Write(pkg len:%d)", len(pkg))
+ }
+ totalSize -= maxPacketLen
+ writeSize += maxPacketLen
+ }
+ lg, err := s.Connection.send(pkg[writeSize:])
Review comment:
@Lvnszn does it need to send if the `totalSize = 0`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo-getty] Lvnszn commented on a change in pull request #82: Opt: split large packet to n*16KB packet
Posted by GitBox <gi...@apache.org>.
Lvnszn commented on a change in pull request #82:
URL: https://github.com/apache/dubbo-getty/pull/82#discussion_r749855691
##########
File path: session.go
##########
@@ -418,11 +422,27 @@ func (s *session) WriteBytes(pkg []byte) (int, error) {
return 0, ErrSessionClosed
}
- lg, err := s.Connection.send(pkg)
+ totalSize := len(pkg)
+ if totalSize == 0 {
+ return 0, nil
+ }
+ writeSize := 0
+ s.wlock.Lock()
+ defer s.wlock.Unlock()
+ for totalSize >= maxPacketLen {
Review comment:
addressed.
##########
File path: session.go
##########
@@ -418,11 +422,27 @@ func (s *session) WriteBytes(pkg []byte) (int, error) {
return 0, ErrSessionClosed
}
- lg, err := s.Connection.send(pkg)
+ totalSize := len(pkg)
+ if totalSize == 0 {
+ return 0, nil
+ }
+ writeSize := 0
+ s.wlock.Lock()
+ defer s.wlock.Unlock()
+ for totalSize >= maxPacketLen {
+ _, err := s.Connection.send(pkg[writeSize:(writeSize + maxPacketLen)])
+ if err != nil {
+ return writeSize, perrors.Wrapf(err, "s.Connection.Write(pkg len:%d)", len(pkg))
+ }
+ totalSize -= maxPacketLen
+ writeSize += maxPacketLen
+ }
+ lg, err := s.Connection.send(pkg[writeSize:])
if err != nil {
- return 0, perrors.Wrapf(err, "s.Connection.Write(pkg len:%d)", len(pkg))
+ return writeSize, perrors.Wrapf(err, "s.Connection.Write(pkg len:%d)", len(pkg))
}
- return lg, nil
+
+ return writeSize + lg, nil
Review comment:
add a var for return, PTAL.
##########
File path: session.go
##########
@@ -418,11 +422,23 @@ func (s *session) WriteBytes(pkg []byte) (int, error) {
return 0, ErrSessionClosed
}
- lg, err := s.Connection.send(pkg)
+ s.wlock.Lock()
+ defer s.wlock.Unlock()
+ totalSize, writeSize := len(pkg), 0
+ for totalSize >= maxPacketLen {
+ _, err := s.Connection.send(pkg[writeSize:(writeSize + maxPacketLen)])
+ if err != nil {
+ return writeSize, perrors.Wrapf(err, "s.Connection.Write(pkg len:%d)", len(pkg))
+ }
+ totalSize -= maxPacketLen
+ writeSize += maxPacketLen
+ }
+ lg, err := s.Connection.send(pkg[writeSize:])
Review comment:
Addressed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo-getty] codecov-commenter edited a comment on pull request #82: Opt: split large packet to n*16KB packet
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #82:
URL: https://github.com/apache/dubbo-getty/pull/82#issuecomment-955414110
# [Codecov](https://codecov.io/gh/apache/dubbo-getty/pull/82?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#82](https://codecov.io/gh/apache/dubbo-getty/pull/82?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (da1e5d5) into [master](https://codecov.io/gh/apache/dubbo-getty/commit/a2461f81a132df705db157b591e00d8535ff6436?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (a2461f8) will **increase** coverage by `0.41%`.
> The diff coverage is `90.90%`.
[![Impacted file tree graph](https://codecov.io/gh/apache/dubbo-getty/pull/82/graphs/tree.svg?width=650&height=150&src=pr&token=WDmUsbxiLS&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/dubbo-getty/pull/82?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #82 +/- ##
==========================================
+ Coverage 68.87% 69.28% +0.41%
==========================================
Files 8 8
Lines 1420 1439 +19
==========================================
+ Hits 978 997 +19
Misses 347 347
Partials 95 95
```
| [Impacted Files](https://codecov.io/gh/apache/dubbo-getty/pull/82?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [session.go](https://codecov.io/gh/apache/dubbo-getty/pull/82/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Vzc2lvbi5nbw==) | `70.25% <90.90%> (+0.80%)` | :arrow_up: |
| [connection.go](https://codecov.io/gh/apache/dubbo-getty/pull/82/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29ubmVjdGlvbi5nbw==) | `80.39% <0.00%> (+0.65%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/dubbo-getty/pull/82?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/dubbo-getty/pull/82?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [a2461f8...da1e5d5](https://codecov.io/gh/apache/dubbo-getty/pull/82?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo-getty] wenxuwan commented on a change in pull request #82: Opt: split large packet to n*16KB packet
Posted by GitBox <gi...@apache.org>.
wenxuwan commented on a change in pull request #82:
URL: https://github.com/apache/dubbo-getty/pull/82#discussion_r750862772
##########
File path: session.go
##########
@@ -418,11 +422,34 @@ func (s *session) WriteBytes(pkg []byte) (int, error) {
return 0, ErrSessionClosed
}
- lg, err := s.Connection.send(pkg)
+ leftPackageSize, totalSize, writeSize := len(pkg), len(pkg), 0
+ if leftPackageSize >= maxPacketLen {
+ s.packetLock.Lock()
Review comment:
only leftPackageSize > maxPacketLen need lock, equal not needed?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo-getty] Lvnszn commented on a change in pull request #82: Opt: split large packet to n*16KB packet
Posted by GitBox <gi...@apache.org>.
Lvnszn commented on a change in pull request #82:
URL: https://github.com/apache/dubbo-getty/pull/82#discussion_r750868122
##########
File path: session.go
##########
@@ -418,11 +422,34 @@ func (s *session) WriteBytes(pkg []byte) (int, error) {
return 0, ErrSessionClosed
}
- lg, err := s.Connection.send(pkg)
+ leftPackageSize, totalSize, writeSize := len(pkg), len(pkg), 0
+ if leftPackageSize >= maxPacketLen {
+ s.packetLock.Lock()
Review comment:
address
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org