Bug 65714 - Expensive File.normalize() in FileUtil.toFileObject()
Expensive File.normalize() in FileUtil.toFileObject()
Status: RESOLVED FIXED
Product: platform
Classification: Unclassified
Component: Filesystems
5.x
All All
: P2 (vote)
: 5.x
Assigned To: rmatous
issues@platform
: API_REVIEW_FAST, PERFORMANCE
Depends on:
Blocks: 65135
  Show dependency treegraph
 
Reported: 2005-10-03 14:45 UTC by _ rkubacki
Modified: 2008-12-23 14:34 UTC (History)
3 users (show)

See Also:
Issue Type: DEFECT
:


Attachments
diff - request for API review + apichanges.xml (2.88 KB, patch)
2005-10-25 13:45 UTC, rmatous
Details | Diff
OK - reworked (3.18 KB, patch)
2005-10-25 16:06 UTC, rmatous
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description _ rkubacki 2005-10-03 14:45:53 UTC
File.normalize() is an expensive call and if we do it during every
FileUtil.toFileObject(). Although we declare that IAE is thrown if not
normalized file is passed as an argument we should make our best effort to avoid
these calls. Probably the check should be performed as assert.

Part of reason why this is slow is that it directly accesses I/O using calls
like File.isDirectory().
Comment 1 _ rkubacki 2005-10-13 09:31:24 UTC
Raising priority to P2. It might need API change but it is desirable to do it
for 5.0. Even the initial intent was to assure that correct args are passed
during development. That's really a job for assertion.
Comment 2 rmatous 2005-10-25 13:45:41 UTC
Created attachment 26315 [details]
diff - request for API review + apichanges.xml
Comment 3 rmatous 2005-10-25 13:50:50 UTC
FileUtil.toFileObject won't throw IllegalArgumentException anymore. More info in
attachment.
Comment 4 Jesse Glick 2005-10-25 14:41:49 UTC
1. You left the "throws IAE" in the method signature.

2. I don't think an assertion is appropriate here, for the usual reasons - never
use assertions to check args on public methods. However I might suggest

boolean asserts = false;
assert asserts = true;
if (asserts && !file.equals(normalizeFile(file))) {
    throw new IllegalArgumentException(...);
}

This would still make sure we catch errors in dev builds, but yield better
performance in release builds. (We *do* measure performance with assertions
disabled, right...?)
Comment 5 rmatous 2005-10-25 16:06:40 UTC
Created attachment 26323 [details]
OK - reworked
Comment 6 _ rkubacki 2005-10-27 08:37:13 UTC
Obviously +1 from me to integrate the patch :-)

To Jesse: yes, we replace netbeans.conf for performance testing to use the same
set of switches as you plan for release.
Comment 7 rmatous 2005-11-02 09:51:17 UTC
Will be integrated.
Comment 8 rmatous 2005-11-02 10:09:07 UTC
/cvs/openide/fs/apichanges.xml,v  <--  apichanges.xml
new revision: 1.5; previous revision: 1.4

/cvs/openide/fs/src/org/openide/filesystems/FileUtil.java,v  <--  FileUtil.java
new revision: 1.11; previous revision: 1.10
Comment 9 Quality Engineering 2008-12-23 14:34:18 UTC
This issue had *1 votes* before move to platform component


By use of this website, you agree to the NetBeans Policies and Terms of Use. © 2012, Oracle Corporation and/or its affiliates. Sponsored by Oracle logo