WordPress.org

Make WordPress Core

Opened 14 months ago

Last modified 3 months ago

#23473 new enhancement

Performance Improvement in three JavaScript Source Codes

Reported by: yongbae Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 3.5
Component: Administration Keywords: has-patch
Focuses: javascript, performance Cc:

Description

By performance profiling in Internet Explorer 10, Firefox 17 and Chrome 25, I found performance improvement in wp-admin/js/post.js, wp-includes/js/media-views.js and wp-includes/js/utils.js. Function clean() at line number 17 in post.js, function set() at line number 98 in util.js and function visibility() at line number 2767 in media-views.js can be improved with same functionality.

I measured inclusive execution times of original code and improved code. The image is the result. The machine I used to measure execution times has Intel Core i5 1.6GHz CPU and 4GB RAM.

I also attached a patch file.

Attachments (2)

graph.png (28.8 KB) - added by yongbae 14 months ago.
Graph of inclusive execution time
performance.patch (2.3 KB) - added by yongbae 14 months ago.

Download all attachments as: .zip

Change History (12)

yongbae14 months ago

Graph of inclusive execution time

yongbae14 months ago

comment:1 SergeyBiryukov14 months ago

  • Component changed from General to Performance
  • Keywords has-patch added

comment:2 scribu14 months ago

So, all 3 improvements amount to less than 1ms, or am I reading the graph wrong?

Version 0, edited 14 months ago by scribu (next)

comment:3 yongbae14 months ago

You are right.

comment:4 scribu14 months ago

  • Keywords close added

Rather than doing micro-optimizations like this with little to no benefit, maybe your time would be better spent fixing all the jQuery deprecation warnings in #22975. Just saying.

comment:5 markoheijnen14 months ago

If everything still works as before I don't see any reason why not to commit this. The patch also cleans out the code a bit.

comment:6 scribu14 months ago

If everything still works as before I don't see any reason why not to commit this.

So, how about we start randomly refactoring code, just for the hell of it? As long as it doesn't break, it's all good, right?

comment:7 markoheijnen14 months ago

We optimize/refactor code pretty often. Like this commit [23410]. I don't say we should pick random code and just change it. That is bad and I can see that. This patch makes the code a little bit more readable too. But that just my opinion. If it gets closed as worksforme I'm fine with that too.

comment:8 follow-up: scribu14 months ago

We rarely refactor purely for aesthetic reasons, though. And, as the graph shows, there simply isn't much performance gain.

comment:9 in reply to: ↑ 8 GaryJ14 months ago

  • Keywords close removed

But, it *is* a performance gain if the graph holds true, the work has already been done, and it's only a dozen or so lines to review for when a committer has got those files at the forefront of their mind. Even my cursory 30-second glance suggest the changes look sensible enough to warrant consideration.

Let's not be dismissive of any efforts to contribute, however small or off-topic you think it might be.

comment:10 nacin3 months ago

  • Component changed from Performance to Administration
  • Focuses javascript performance added
Note: See TracTickets for help on using tickets.