You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@poi.apache.org by mo...@gmail.com, mo...@gmail.com on 2018/08/18 20:22:27 UTC

XSSFCellBorder.equals

Hi,
A bit surprised by the inefficiency of some elements in XSSF.

Take for example XSSFCellBorder.equals method:

    public boolean equals(Object o) {
        if (!(o instanceof XSSFCellBorder)) return false;

        XSSFCellBorder cf = (XSSFCellBorder) o;
        return border.toString().equals(cf.getCTBorder().toString());
    }

I created an .xlsx file with 12000+ cells, with borders. This means the library calls StylesTable.putBorder some 96000 times (12000 * 4 * 2, 12000 cells, 4 borders, and 2 calls per border style + color)

putBorder calls "indexOf" on the borders list, and so indexOf calls XSSFCellBorder.equals for every element in the borders list.

The equals method tests 2 strings constructed from complex XML objects.

Writing the file takes 3 minutes !

With SmartXLS it takes less than 3 seconds.

Do you think this behavior could be modified somehow ?

Regards
FD

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org


Re: XSSFCellBorder.equals

Posted by mo...@gmail.com, mo...@gmail.com.
Hi again,

I apologize for not doing it the right way. I'll paste the modifications I did hoping it would be clear:

In StylesTable.java

Add this variable:
private final Map<String, Integer> bordersIndices = new HashMap();

The method putBorder becomes:

    public int putBorder(XSSFCellBorder border) {
      Object key = border.getKey();
      Integer index = this.bordersIndices.get(key);
      if (index != null) return index;
//        int idx = borders.indexOf(border);
//        if (idx != -1) {
//            return idx;
//        }
        borders.add(border);
        border.setThemesTable(theme);
        index = borders.size() - 1;
        this.bordersIndices.put(key, index);
        return index;
//        return borders.size() - 1;
    }

In XSSFCellBorder.java

Add these:

String key = ""; //To avoid null tests

public String getKey() { return this.key; }
private void updateKey()  {   this.key = this.border.toString();  }

Modify these by calling this.updateKey():

    public XSSFCellBorder(CTBorder border, IndexedColorMap colorMap) {
        this.border = border;
        this._indexedColorMap = colorMap;
        this.updateKey();
    }
    public void setBorderStyle(BorderSide side, BorderStyle style) {
        getBorder(side, true).setStyle(STBorderStyle.Enum.forInt(style.ordinal() + 1));
        this.updateKey();
    }

    public void setBorderColor(BorderSide side, XSSFColor color) {
        CTBorderPr borderPr = getBorder(side, true);
        if (color == null) borderPr.unsetColor();
        else
            borderPr.setColor(color.getCTColor());
        this.updateKey();
    }

    private CTBorderPr getBorder(BorderSide side, boolean ensure) {
        CTBorderPr borderPr;
        switch (side) {
            case TOP:
                borderPr = border.getTop();
                if (ensure && borderPr == null)
                {
                  borderPr = border.addNewTop();
                  this.updateKey();
                }
                break;
            case RIGHT:
                borderPr = border.getRight();
                if (ensure && borderPr == null)
                {
                  borderPr = border.addNewRight();
                  this.updateKey();
                }
                break;
            case BOTTOM:
                borderPr = border.getBottom();
                if (ensure && borderPr == null)
                {
                  borderPr = border.addNewBottom();
                  this.updateKey();
                }
                break;
            case LEFT:
                borderPr = border.getLeft();
                if (ensure && borderPr == null)
                {
                  borderPr = border.addNewLeft();
                  this.updateKey();
                }
                break;
            default:
                throw new IllegalArgumentException("No suitable side specified for the border");
        }
        return borderPr;
    }

Modify these:

    public int hashCode() {
//        return border.toString().hashCode();
        return this.key.hashCode();
    }

    public boolean equals(Object o) {
        if (!(o instanceof XSSFCellBorder)) return false;

        XSSFCellBorder cf = (XSSFCellBorder) o;
//        return border.toString().equals(cf.getCTBorder().toString());
        return this.key.equals(cf.key);
    }



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org


Re: XSSFCellBorder.equals

Posted by Dominik Stadler <do...@gmx.at>.
Hi,

Yes, would be very interesting how you solved it, can you attach it as a
patch to the bug-entry?

Thanks... Dominik.

On Thu, Aug 23, 2018 at 11:18 PM, monnomiznogoud@gmail.com <
monnomiznogoud@gmail.com> wrote:

> Hi Dominik,
>
> I've already implemented a solution. I've modified 2 files
> StylesTable.java and XSSFCellBorder.java
>
> A simple solution really. If you're interested I could send you the
> modified files.
>
> FD
>
> On 2018/08/23 08:55:54, Dominik Stadler <do...@gmx.at> wrote:
> > Yes, this is inefficient in some cases, this is already handled in
> > https://bz.apache.org/bugzilla/show_bug.cgi?id=54593 there were some
> > discussion about possible changes but unfortunately no full solution yet.
> >
> > Dominik.
> >
> > On Sat, Aug 18, 2018 at 10:22 PM monnomiznogoud@gmail.com <
> > monnomiznogoud@gmail.com> wrote:
> >
> > > Hi,
> > > A bit surprised by the inefficiency of some elements in XSSF.
> > >
> > > Take for example XSSFCellBorder.equals method:
> > >
> > >     public boolean equals(Object o) {
> > >         if (!(o instanceof XSSFCellBorder)) return false;
> > >
> > >         XSSFCellBorder cf = (XSSFCellBorder) o;
> > >         return border.toString().equals(cf.getCTBorder().toString());
> > >     }
> > >
> > > I created an .xlsx file with 12000+ cells, with borders. This means the
> > > library calls StylesTable.putBorder some 96000 times (12000 * 4 * 2,
> 12000
> > > cells, 4 borders, and 2 calls per border style + color)
> > >
> > > putBorder calls "indexOf" on the borders list, and so indexOf calls
> > > XSSFCellBorder.equals for every element in the borders list.
> > >
> > > The equals method tests 2 strings constructed from complex XML objects.
> > >
> > > Writing the file takes 3 minutes !
> > >
> > > With SmartXLS it takes less than 3 seconds.
> > >
> > > Do you think this behavior could be modified somehow ?
> > >
> > > Regards
> > > FD
> > >
> > > ---------------------------------------------------------------------
> > > To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
> > > For additional commands, e-mail: dev-help@poi.apache.org
> > >
> > >
> >
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
> For additional commands, e-mail: dev-help@poi.apache.org
>
>

Re: XSSFCellBorder.equals

Posted by mo...@gmail.com, mo...@gmail.com.
Hi Dominik,

I've already implemented a solution. I've modified 2 files StylesTable.java and XSSFCellBorder.java

A simple solution really. If you're interested I could send you the modified files.

FD

On 2018/08/23 08:55:54, Dominik Stadler <do...@gmx.at> wrote: 
> Yes, this is inefficient in some cases, this is already handled in
> https://bz.apache.org/bugzilla/show_bug.cgi?id=54593 there were some
> discussion about possible changes but unfortunately no full solution yet.
> 
> Dominik.
> 
> On Sat, Aug 18, 2018 at 10:22 PM monnomiznogoud@gmail.com <
> monnomiznogoud@gmail.com> wrote:
> 
> > Hi,
> > A bit surprised by the inefficiency of some elements in XSSF.
> >
> > Take for example XSSFCellBorder.equals method:
> >
> >     public boolean equals(Object o) {
> >         if (!(o instanceof XSSFCellBorder)) return false;
> >
> >         XSSFCellBorder cf = (XSSFCellBorder) o;
> >         return border.toString().equals(cf.getCTBorder().toString());
> >     }
> >
> > I created an .xlsx file with 12000+ cells, with borders. This means the
> > library calls StylesTable.putBorder some 96000 times (12000 * 4 * 2, 12000
> > cells, 4 borders, and 2 calls per border style + color)
> >
> > putBorder calls "indexOf" on the borders list, and so indexOf calls
> > XSSFCellBorder.equals for every element in the borders list.
> >
> > The equals method tests 2 strings constructed from complex XML objects.
> >
> > Writing the file takes 3 minutes !
> >
> > With SmartXLS it takes less than 3 seconds.
> >
> > Do you think this behavior could be modified somehow ?
> >
> > Regards
> > FD
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
> > For additional commands, e-mail: dev-help@poi.apache.org
> >
> >
> 

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org


Re: XSSFCellBorder.equals

Posted by Dominik Stadler <do...@gmx.at>.
Yes, this is inefficient in some cases, this is already handled in
https://bz.apache.org/bugzilla/show_bug.cgi?id=54593 there were some
discussion about possible changes but unfortunately no full solution yet.

Dominik.

On Sat, Aug 18, 2018 at 10:22 PM monnomiznogoud@gmail.com <
monnomiznogoud@gmail.com> wrote:

> Hi,
> A bit surprised by the inefficiency of some elements in XSSF.
>
> Take for example XSSFCellBorder.equals method:
>
>     public boolean equals(Object o) {
>         if (!(o instanceof XSSFCellBorder)) return false;
>
>         XSSFCellBorder cf = (XSSFCellBorder) o;
>         return border.toString().equals(cf.getCTBorder().toString());
>     }
>
> I created an .xlsx file with 12000+ cells, with borders. This means the
> library calls StylesTable.putBorder some 96000 times (12000 * 4 * 2, 12000
> cells, 4 borders, and 2 calls per border style + color)
>
> putBorder calls "indexOf" on the borders list, and so indexOf calls
> XSSFCellBorder.equals for every element in the borders list.
>
> The equals method tests 2 strings constructed from complex XML objects.
>
> Writing the file takes 3 minutes !
>
> With SmartXLS it takes less than 3 seconds.
>
> Do you think this behavior could be modified somehow ?
>
> Regards
> FD
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
> For additional commands, e-mail: dev-help@poi.apache.org
>
>