Bug 99311 - Detect SSDs in pagein
Summary: Detect SSDs in pagein
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: LibreOffice (show other bugs)
Version:
(earliest affected)
unspecified
Hardware: All Linux (All)
: medium normal
Assignee: Not Assigned
URL:
Whiteboard: target:5.2.0
Keywords: difficultyBeginner, easyHack, skillCpp, topicCleanup
Depends on:
Blocks:
 
Reported: 2016-04-14 23:42 UTC by Björn Michaelsen
Modified: 2017-02-14 08:57 UTC (History)
3 users (show)

See Also:
Crash report or crash signature:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Björn Michaelsen 2016-04-14 23:42:12 UTC
The original Pagein was intended to speed up starting LibreOffice. On SSDs -- which is the majority of disks these days -- its actually counterproductive, also it has pointless code like:
 p[strlen(p)] = '\0';
at desktop/unx/source/pagein.c:57 -- which, at best, is taking a long time to do nothing.

So this EasyHack is to remove Pagein completely from LibreOffice. A starting point is:

 http://opengrok.libreoffice.org/search?q=Pagein&project=core&defs=&refs=&path=&hist=

desktop/, desktop/unx/source/, solenv/gbuild/, scp2/source/ooo/, bin/distro-install-file-lists, Makefile.in all need to be cleaned up. Stuff in sal/osl/ is unrelated and should be left alone.

related discussion on: https://gerrit.libreoffice.org/#/c/23872/
Comment 1 Michael Meeks 2016-04-30 08:13:02 UTC
Hmm; so there are still hard disks out there =) my daughters' laptops all have them. I'd prefer to instead detect the presence of an SSD and then bail out. That turns out to be not impossible ;-)

cat /sys/block/sda/queue/rotational

is 0 for SSD and 1 for slow hard-disks =)

So - we need to detect which device this is on - I guess we want to do:

http://linux.die.net/man/2/fstat

on each shared library - or perhaps just once on the top-level program/ directory  - and then check if this is on a rotational media.
Comment 2 Michael Meeks 2016-04-30 08:23:49 UTC
On Linux - and we can make this very linux-only with #ifdef LINUX - we have:

/sys/dev/block/

And we can construct a file-name from the major / minor device id details from the dev_t we get from fstat.

So some small piece of code to detect if this is an SSD should be -reasonably- easy =)
Comment 3 NURHAK ALTIN 2016-05-01 12:54:53 UTC
Patch is on gerrit =)

Bug is closed..
Comment 4 jani 2016-05-01 13:56:11 UTC
(In reply to NURHAK ALTIN from comment #3)
> Patch is on gerrit =)
> 
> Bug is closed..

Well not until your patch is merged (or commented).

rgds
jan i.
Comment 5 Commit Notification 2016-05-01 15:49:35 UTC
Nurhak ALTIN committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=a42169cdae80f88e1c4b52c333e928d239d917f5

tdf#99311 Detect SSDs in pagein

It will be available in 5.2.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 6 Björn Michaelsen 2016-05-03 08:46:52 UTC
(In reply to Michael Meeks from comment #1)
> Hmm; so there are still hard disks out there =) my daughters' laptops all
> have them.

Yes, but so far there is no -- not even anecdotal -- evidence that pagein is of any help at all even for spinning rust.

Given that this stuff is antique and questionale C89 code we even have special cased build targets for, without that, its hard to justify keeping this stuff around. There are a bazillion other issues with this code:
- hardcoded buffer sizes and no checking of overflows
- "paging in" aborting on first non found lib, but not caring for packaging causing files to be missing
- does it even handle libmerged?
- ... lots and lots more, if you just look at it: the code smells badly

All of those cause fragility and subtle performance hits in the real world, doing more bad than good. So I would still vote for this stuff to just die -- or alternatively, if you want to put it in a strict test harness with good coverage of all the permutations, do that. But note that even the most common use case of this (cold start from spinning rust) is already quite uncommon these days.
Comment 7 Björn Michaelsen 2016-05-28 13:48:56 UTC
(In reply to Commit Notification from comment #5)
> Nurhak ALTIN committed a patch related to this issue.
> It has been pushed to "master":

Thus closing this one.