Make WordPress Core

Opened 2 years ago

Last modified 14 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:


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 2 years ago.
Graph of inclusive execution time
performance.patch (2.3 KB) - added by yongbae 2 years ago.

Download all attachments as: .zip

Change History (12)

@yongbae2 years ago

Graph of inclusive execution time

@yongbae2 years ago

comment:1 @SergeyBiryukov2 years ago

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

comment:2 @scribu2 years ago

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

Version 0, edited 2 years ago by scribu (next)

comment:3 @yongbae2 years ago

You are right.

comment:4 @scribu2 years 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 @markoheijnen2 years 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 @scribu2 years 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 @markoheijnen2 years 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: @scribu2 years 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 @GaryJ2 years 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 @nacin14 months ago

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