[
https://issues.apache.org/jira/browse/ODFTOOLKIT-426?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15244224#comment-15244224
]
Nimarukan commented on ODFTOOLKIT-426:
--------------------------------------
Thank you for the checkin!
A couple suggestions:
1. Commit comment issue number as well as title/summary:
svn log -r 1738440
--> Although you include the issue TITLE/summary, suggest also including the
issue NUMBER in the commit comment, so someone looking at blame or log can more
easily find the original issue discussion for more background on the change.
(Sometimes issue titles change. The issue number might also change, if the
project moves to a new database. So the suggestion is to include both.)
(TortoiseSVN can be used to change SVN log comments; there must be other ways
also.)
2. Preserve original line ends
The code is there, however, it appears your tooling converted all the LF line
breaks to CRLF.
A diff shows all the lines changed, rather than just the fixed lines.
svn diff -c 1738440 > diff.patch
A blame no longer shows the older revision numbers when the text of the other
lines was last changed.
svn blame odf/simple/src/main/java/org/odftoolkit/simple/Document.java >
blame.txt
--> Maybe you can restore the blame history somehow, maybe like this:
0. cd odf/simple/src/main/java/org/odftoolkit/simple
1. Read the file into an editor (or make a copy).
2. Restore the previous version (with svn blame info for each line)
svn update --force -r 1738439 Document.java
3. Overwrite with the copy in the ditor (or copied file)
If you can specify the line ends in your editor, specify the original
unix-style LF line ends before overwriting the file.
Otherwise, find a utility like dos2unix to convert line ends.
dos2unix Document.java
4. Verify diff now shows only changed regions:
svn diff > diff.patch
5. verify blame now shows original revision number except for changed lines:
svn blame Document.java > blame.txt
6. Commit the change
svn ci -m "ODFTOOKIT-426: PERFORMANCE: simple Document.getCopyStyleList
improvements to reduce String allocation, hashing, etc. (patch by Nimarukan)
(restore blame info and line ends)"
(Regarding whether to copy all styles: If the document was a template or a
form, users might want all styles preserved so they are available for editing.
But you're right it seems strange to copy all of one kind and not another.
Possible future issue: If merging is important, someone may want to merge
identical styles rather than duplicate them.)
> PERFORMANCE: simple Document.getCopyStyleList improvements to reduce String
> allocation, hashing, etc.
> -----------------------------------------------------------------------------------------------------
>
> Key: ODFTOOLKIT-426
> URL: https://issues.apache.org/jira/browse/ODFTOOLKIT-426
> Project: ODF Toolkit
> Issue Type: Improvement
> Components: simple api
> Environment: 0.8.2-incubating-SNAPSHOT, jdk1.8.0_77, win7
> Reporter: Nimarukan
> Priority: Minor
> Labels: patch, performance
> Attachments: Issue-426-patches.zip,
> result-ods-xml-before-vs-after-patches_diff.zip
>
>
> org.odftoolkit.simple
> Document.getCopyStyleList
> is a top hotspot in a (NetBeans) sampled profiling run of test_1 of
> https://issues.apache.org/jira/secure/attachment/12795379/odftoolkit-performance-test.zip
> When a sheet is copied from one document to another, simple.Document also
> merges metadata such as styles. For each style it iterates over every cell
> with that style. The current code can be improved by reusing constructed
> strings (also reducing garbage collection), removing unnecessary hash table
> lookups, etc.
> After these changes, Document.getCopyStyleList takes about 1/16th the
> original time in the profiled run.
> (This is surprising. Maybe it enables some optimizations that eliminate
> String creation in an inner loop in common cases.)
> (The gain is not due to an error of omission: The template.ods source file
> used in this performance test is a spreadsheet with styled cells. The
> differences between the ods xml files of the result spreadsheet produced
> before these patches are applied, and the ods xml files of the result
> spreadsheet produced after these patches are applied, are only in times and
> in random strings appended to style names. So these changes did not
> significantly change the result spreadsheet.)
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
