To comment on the following update, log in, then open the issue:
http://www.openoffice.org/issues/show_bug.cgi?id=56783
------- Additional comments from [EMAIL PROTECTED] Fri Oct 28 00:23:11 -0700
2005 -------
comments from jb:
> diff -u -p -u -r1.3 localfilehelper.cxx
> --- configmgr/source/localbe/localfilehelper.cxx 16 Feb 2005 16:46:51 -0000
1.3
> +++ configmgr/source/localbe/localfilehelper.cxx 27 Oct 2005 11:21:18 -0000
> @@ -126,6 +128,10 @@ namespace configmgr if (_sURL.getLength() == 0)
> return false;
>
> + // This method has no right to be so under-performing to
> + // achieve so, so little of any usefulness.
> + return true;
> +
> DirectoryItem aDirItem;
This is function normalizeURL(). AFAICT we call it only for directories where we
need to compare directory names. It is needed there (for example) when
non-case-sensitive filesystems are involved (need to ask the sal/osl crowd what
other reasons can cause a different file URL to be reported).
I believe we can eliminate some redundant stats here with additional bookkeeping
- but we can't get rid of that in general. We might also add a configmgrrc
setting to skip this, if it is known that no file systems where
osl_getFileStatus can produce a URL that is different from the input are
involved, but the default for deploying to unknown environments would have to
stay unchanged.
> diff -u -p -u -r1.5 localmultistratum.cxx
> --- configmgr/source/localbe/localmultistratum.cxx 8 Sep 2005 04:04:46
-0000 1.5
> +++ configmgr/source/localbe/localmultistratum.cxx 27 Oct 2005 11:21:18
> -0000
> @@ -107,7 +107,7 @@ uno::Sequence< rtl::OUString > SAL_CALL rtl::OUString
const aComponentUrl = aLayerUrl + componentToPath(aComponent);
> using namespace osl;
> - const sal_uInt32 k_STATUS_FIELDS = FileStatusMask_Type |
FileStatusMask_FileName; + const sal_uInt32 k_STATUS_FIELDS =
FileStatusMask_FileName; Directory aComponentDirectory(aComponentUrl);
> DirectoryItem aItem;
> std::vector< rtl::OUString > aResult;
> @@ -132,13 +132,12 @@ uno::Sequence< rtl::OUString > SAL_CALL
OSL_TRACE("Reading Component Directory - Error (%u) getting status of directory
item.\n", unsigned(errcode));
> break;
> }
> -
> - OSL_ENSURE( aItemDescriptor.isValid(FileStatusMask_Type), "Could
not get type of directory item");
> - if (aItemDescriptor.getFileType() != FileStatus::Regular)
> - continue;
> OSL_ENSURE(
aItemDescriptor.isValid(FileStatusMask_FileName), "Could not get Name of
component found");
> OUString const aFileName = aItemDescriptor.getFileName();
> +
> + // It is reasonable to assume a .xcu file is not a directory & =>
> + // not stat each directory entry at considerable cost.
> if
(!aFileName.endsWithIgnoreAsciiCaseAsciiL(RTL_CONSTASCII_STRINGPARAM(kLocalDataSuffix)))
Fair enough. The .xcu check should be sufficient. This has the added effect that
.xcu files may be symlinks - which also sounds reasonable.
> diff -u -p -u -r1.16 filehelper.cxx
> --- configmgr/source/misc/filehelper.cxx 8 Sep 2005 04:09:44 -0000 1.16
> +++ configmgr/source/misc/filehelper.cxx 27 Oct 2005 11:21:18 -0000
> @@ -143,9 +143,22 @@ namespace configmgr static const TimeValue
k_NullTime = {0,0};
> sal_uInt64 aSize = 0;
> rModifyTime = k_NullTime;
> + rtl::OUString aURL;
>
> - DirectoryItem aItem;
> - if (osl::FileBase::E_None == DirectoryItem::get(_sURL, aItem))
> + DirectoryItem aItem;
> +
> +#ifndef STAT_LOTS_OF_UNUSED_FILES
> + // Statting every file takes way to long - we can use the directory
> + // time-stamp & size instead, if we need to provoke an update
> + // a simple cat > foo; rm foo will do that.
> + // The dir size is (hopefully) non-0 and a good enough proxy value.
> +
> + aURL = FileHelper::getParentDir(_sURL);
> +#else
> + aURL = _sURL;
> +#endif
> +
> + if (osl::FileBase::E_None == DirectoryItem::get(aURL, aItem))
> {
> FileStatus
aStatus(osl_FileStatus_Mask_ModifyTime|osl_FileStatus_Mask_Type|osl_FileStatus_Mask_FileSize);
> if (osl::FileBase::E_None == aItem.getFileStatus(aStatus))
I don't think that is acceptable.
Firstly, it violates the semantics of that function. If anybody really wanted to
do this, the parent directory substitution should be in the caller (i.e. in the
getTimestamp() method in localfilelayer.cxx).
Secondly it does not work everywhere. The semantics of that 'timestamp' is that
it must change, if the file changes. You can't guarantee that. Without actively
provoking an update, changes might go completely unnoticed on most filesystems.
Generally I'm wary of directory timestamps. Is their behaviour really portable
across all kinds of file systems and OSs?
For directory sizes that certainly isn't the case. For example a quick check on
solaris revealed that there (at least on ufs and nfs) directory sizes are
integral multiples of 512. And I can't tell quickly how Windows behaves in that
regard ("dir" doesn't even show file size for directories).
BTW: The size check was added to make sure that even the case where files are
touched to a fixed modification time (done by some installers) is caught.
My experience is that requiring explicit actions to invalidate or refresh caches
after modifying the source data is error-prone. You can only convey that to
users through documentation - which many won't find or read.
There is a safe patch that ought to reduce the number of getTimestamp calls
significantly (see below). But I don't think it helps as much as one could hope,
as it probably won't reduce the number of unique stats significantly. The stats
done for isMoreRecent are unnecessary most of the time, but the same files will
be stat'ed by the binary cache ...
Index: localstratumbase.cxx
===================================================================
RCS file: /cvs/util/configmgr/source/localbe/localstratumbase.cxx,v
retrieving revision 1.5
diff -u -r1.5 localstratumbase.cxx
--- localstratumbase.cxx 8 Sep 2005 04:07:18 -0000 1.5
+++ localstratumbase.cxx 27 Oct 2005 14:37:19 -0000
@@ -149,6 +149,10 @@
sal_Bool LocalStratumBase::isMoreRecent(const rtl::OUString& aFileUrl,
const rtl::OUString& aTimestamp)
{
+ // if reference timestamp is empty, every layer needs to be read
+ if (aTimestamp.getLength() == 0)
+ return true;
+
rtl::OUString layerUrl ;
rtl::OUString subLayerUrl ;
---------------------------------------------------------------------
Please do not reply to this automatically generated notification from
Issue Tracker. Please log onto the website and enter your comments.
http://qa.openoffice.org/issue_handling/project_issues.html#notification
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
