Differential D17048

Restore sendfile support
ClosedPublic
Actions

Authored by davidedmundson on Nov 20 2018, 1:38 PM.

Details

Reviewers
dfaure
Commits
R241:31980ecd1cac: Restore sendfile support
Summary

Somehow in the kdelibs -> framework port the cmake checks for
HAVE_SENDFILE got lost.

That re-enables a massive optimisation in the file kioslave that has all the code existing and used in kdelibs4 that we're currently missing.

Test Plan

Put a compilation fail inside the #ifdef, before it wasn't triggered, now it is.

Ran unit tests
Moved a file in dolphin

Diff Detail

Repository
R241 KIO
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 5183
Build 5201: arc lint + arc unit
davidedmundson created this revision.Nov 20 2018, 1:38 PM
Restricted Application added a project: Frameworks. · View Herald TranscriptNov 20 2018, 1:38 PM
Restricted Application added a subscriber: kde-frameworks-devel. · View Herald Transcript
davidedmundson requested review of this revision.Nov 20 2018, 1:38 PM
davidedmundson edited the summary of this revision. (Show Details)Nov 20 2018, 1:42 PM
apol added a subscriber: apol.Nov 20 2018, 1:49 PM

+1 looks good to me.

Chenge #if defined HAVE_SENDFILE to #if HAVE_SENDFILE though, cmakedefine01 will define it always, if I'm not mistaken.

ngraham added a subscriber: ngraham.Nov 20 2018, 3:18 PM

Does this result in a performance improvement? Anything quantifiable I can blog about?

davidedmundson added a comment.Nov 20 2018, 3:42 PM

It should be faster.

It'll go from reading a file into a buffer in userspace and writing it back to it all being handled automagically.

I'm not sure you can really blog about it, the code existed since 2003.

dfaure accepted this revision.Nov 20 2018, 10:04 PM

Nice catch.

... and catching this is the whole reason for #if instead of #ifdef ... too bad this one was still using ifdef (if defined).

This revision is now accepted and ready to land.Nov 20 2018, 10:04 PM
Closed by commit R241:31980ecd1cac: Restore sendfile support (authored by davidedmundson). · Explain WhyNov 21 2018, 3:31 PM
This revision was automatically updated to reflect the committed changes.
sefaeyeoglu added a subscriber: sefaeyeoglu.Nov 25 2018, 3:42 PM

Can someone benchmark this? Would be nice to know how much this affects performance.

Showing Only Differences

This revision modifies 2 more files that are hidden because they were not modified between selected diffs and they have no inline comments.

Revision Contents
Changeset List

PathPackages
Msrc/ioslaves/file/file_unix.cpp (2 lines)

Diff 45889

View Options

src/ioslaves/file/file_unix.cpp

Loading...